Skip to content

Conversation

@ide
Copy link
Contributor

@ide ide commented Jan 11, 2016

The current versions in these files is 0.12.0, which is out of date. Better to claim no version than the wrong version, so this diff changes the versions to 0.0.0-master.

Release branches will still have the correct versions.

…0-master

The current versions in these files is 0.12.0, which is out of date. Better to claim no version than the wrong version, so this diff changes the versions to 0.0.0-master.

Release branches will still have the correct versions.
@ide
Copy link
Contributor Author

ide commented Jan 11, 2016

@facebook-github-bot shipit

cc @vjeux

@facebook-github-bot
Copy link
Contributor

By analyzing the blame information on this pull request, we identified @ide, @mkonicek and @vjeux to be potential reviewers.

@facebook-github-bot facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Jan 11, 2016
@vjeux
Copy link
Contributor

vjeux commented Jan 11, 2016

Thx :)

@brentvatne
Copy link
Collaborator

👍 👍

@vjeux
Copy link
Contributor

vjeux commented Jan 11, 2016

@facebook-github-bot shipit

@facebook-github-bot
Copy link
Contributor

Thanks for importing. If you are an FB employee go to https://our.intern.facebook.com/intern/opensource/github/pull_request/704689026334067/int_phab to review.

@vjeux
Copy link
Contributor

vjeux commented Jan 11, 2016

There must be a bug in my multi-line handling, seems like it doesn't always catch it :(

@ide
Copy link
Contributor Author

ide commented Jan 11, 2016

Missing a /g flag maybe?

@vjeux
Copy link
Contributor

vjeux commented Jan 11, 2016

    $prefix = '@facebook-github-bot ';

    $unchecked_commands = array();
    foreach ($comments as $comment) {
      $body = $comment['body'];
      foreach (Str::explode("\n", $body) as $line) {
        if (Str::startsWith($line, $prefix)) {
          $unchecked_commands[] = array(
            'body' => Str::substr($line, Str::len($prefix)),
            'author' => $comment['author'],
          );
        }
      }
    }

I don't see anything off just by staring at it :(

@ide
Copy link
Contributor Author

ide commented Jan 12, 2016

It looks fine to me too.

@vjeux
Copy link
Contributor

vjeux commented Jan 12, 2016

@facebook-github-bot shipit

@facebook-github-bot
Copy link
Contributor

Thanks for importing. If you are an FB employee go to https://our.intern.facebook.com/intern/opensource/github/pull_request/704689026334067/int_phab to review.

@Bhullnatik
Copy link
Contributor

@ghost ghost closed this in 66cd8f7 Jan 13, 2016
@skevy
Copy link
Contributor

skevy commented Jan 13, 2016

@ide random FYI

This commit broke pod install for me...it errors with:

[!] Unable to satisfy the following requirements:

- `React/RCTText (from `../node_modules/react-native`)` required by `Podfile`
- `React/RCTText (from `../node_modules/react-native`)` required by `Podfile`
- `React/RCTText (from `../node_modules/react-native`)` required by `Podfile`

This happens even when I remove the "Pods" directory and try again. It looks like 0.0.0-master isn't going to work for the podspec. :(

@ide ide deleted the ver0 branch March 11, 2016 08:42
ghost pushed a commit that referenced this pull request Mar 29, 2016
Summary:In #5241 ide updated the version number to be a fake one so that people wouldn't send in PRs just bumping the version.

Unfortunately, this leads to compatibility issues when developing against `master` with 3rd-party components that declare React Native as a `peerDependency`. For example, I'm using [react-native-orientation](https://github.com/yamill/react-native-orientation) which has a peerDependency of `"react-native": ">=0.5"`.

I see a few ways to deal with this:

1. Only develop against the releases on npm, not git snapshots.
2. Ask ecosystem projects to not include a minimum version of `react-native` in their peerDependencies.
3. Track the RN release numbers in the git repository (eg it would be 0.19 right now).
4. Make the release number on master huge (1000 in this PR) so it's obviously a fake number but will still comply with >= checks.

I don't think option 2 is good because it's reasonable for a package author to want to specify a minimum R
Closes #5556

Differential Revision: D3110274

fb-gh-sync-id: 8638157d44ee99945337fbf585936b50699f0341
fbshipit-source-id: 8638157d44ee99945337fbf585936b50699f0341
zebulgar pushed a commit to nightingale/react-native that referenced this pull request Jun 18, 2016
Summary:In facebook#5241 ide updated the version number to be a fake one so that people wouldn't send in PRs just bumping the version.

Unfortunately, this leads to compatibility issues when developing against `master` with 3rd-party components that declare React Native as a `peerDependency`. For example, I'm using [react-native-orientation](https://github.com/yamill/react-native-orientation) which has a peerDependency of `"react-native": ">=0.5"`.

I see a few ways to deal with this:

1. Only develop against the releases on npm, not git snapshots.
2. Ask ecosystem projects to not include a minimum version of `react-native` in their peerDependencies.
3. Track the RN release numbers in the git repository (eg it would be 0.19 right now).
4. Make the release number on master huge (1000 in this PR) so it's obviously a fake number but will still comply with >= checks.

I don't think option 2 is good because it's reasonable for a package author to want to specify a minimum R
Closes facebook#5556

Differential Revision: D3110274

fb-gh-sync-id: 8638157d44ee99945337fbf585936b50699f0341
fbshipit-source-id: 8638157d44ee99945337fbf585936b50699f0341
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants