Skip to content

Conversation

@satya164
Copy link
Contributor

Using local version of the Flow make it easy to have project specific version instead of relying on the user to have the correct version installed globally. For example, React Native uses an older version of Flow, while I have the latest version installed. Now I cannot typecheck the code because my version doesn't match the .flowconfig.

Test plan (required)

Run npm run lint and npm run flow to run eslint and flow.

cc @bestander @mkonicek

@facebook-github-bot
Copy link
Contributor

By analyzing the blame information on this pull request, we identified @spicyj, @cpojer 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 Feb 25, 2016
@satya164 satya164 changed the title Use local version of tools, update ESLint Use local version of tools, update ESLint to 2.0 Feb 25, 2016
@bestander
Copy link
Contributor

@facebook-github-bot import

@facebook-github-bot
Copy link
Contributor

Thanks for importing. If you are an FB employee go to Phabricator to review.

package.json Outdated
"scripts": {
"test": "NODE_ENV=test jest",
"lint": "eslint Examples/ Libraries/",
"test": "NODE_ENV=test node_modules/jest-cli/bin/jest.js",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this shouldn't be necessary? npm install creates a .bin folder in node_modules with a jest executable. npm run adds this to the $PATH.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cpojer My bad. For some reason it didn't work for me last time I tried. Will update the PR.

@cpojer
Copy link
Contributor

cpojer commented Feb 25, 2016

Can you clarify the "use local tools"? Your explanation doesn't make sense in the context of how npm works. It might make sense for flow, but not for jest or eslint.

@satya164
Copy link
Contributor Author

@cpojer Updated!

@facebook-github-bot
Copy link
Contributor

@satya164 updated the pull request.

@satya164 satya164 changed the title Use local version of tools, update ESLint to 2.0 Use flow-bin to typecheck locally, update ESLint to 2.0 Feb 25, 2016
@bestander
Copy link
Contributor

Waiting for @cpojer to approve this.
I'll reimport, because this touches npm deps, I'll need to update internal npm deps with a script.

@cpojer
Copy link
Contributor

cpojer commented Feb 25, 2016

Yeah I think this makes more sense.

@mkonicek
Copy link
Contributor

Thanks for doing this!

Please hold on with merging this until Travis tests are back to green to be safe.

@bestander
Copy link
Contributor

Added tag so that we come back to this next week

@bestander
Copy link
Contributor

@satya164 maybe we can remove global flow install in Travis then?
https://github.com/facebook/react-native/blob/master/.travis.yml#L18

@satya164
Copy link
Contributor Author

@bestander done!

@facebook-github-bot
Copy link
Contributor

@satya164 updated the pull request.

1 similar comment
@facebook-github-bot
Copy link
Contributor

@satya164 updated the pull request.

@facebook-github-bot
Copy link
Contributor

@satya164 updated the pull request.

@mkonicek
Copy link
Contributor

mkonicek commented Mar 1, 2016

e2e tests are still broken so we can't merge this yet, thanks for the patience.

@bestander
Copy link
Contributor

@facebook-github-bot import

@facebook-github-bot
Copy link
Contributor

Thanks for importing. If you are an FB employee go to Phabricator to review.

@bestander
Copy link
Contributor

shipping...

@ghost ghost closed this in 36f1961 Mar 3, 2016
pglotov pushed a commit to pglotov/react-native that referenced this pull request Mar 15, 2016
Summary:Using local version of the Flow make it easy to have project specific version instead of relying on the user to have the correct version installed globally. For example, React Native uses an older version of Flow, while I have the latest version installed. Now I cannot typecheck the code because my version doesn't match the `.flowconfig`.

**Test plan (required)**

Run `npm run lint` and `npm run flow` to run `eslint` and `flow`.

cc bestander mkonicek
Closes facebook#6145

Reviewed By: dmmiller

Differential Revision: D2976616

Pulled By: bestander

fb-gh-sync-id: bb08f6f8ceb09f644ec1d45c40b4cb7a9d3cfef5
shipit-source-id: bb08f6f8ceb09f644ec1d45c40b4cb7a9d3cfef5
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.

5 participants