Skip to content

Extract out CLI #22337

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
wants to merge 3 commits into from
Closed

Extract out CLI #22337

wants to merge 3 commits into from

Conversation

grabbou
Copy link
Contributor

@grabbou grabbou commented Nov 18, 2018

Continuation of #22174 with an exception that local-cli folder is left in React Native repository to keep Facebook internal and React Native calls still working.

Separate strategy should be developed to remove all uses of local-cli in favor of dedicated utilities.

@grabbou grabbou requested a review from hramos as a code owner November 18, 2018 17:33
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 18, 2018
@grabbou
Copy link
Contributor Author

grabbou commented Nov 18, 2018

Few questions:

  1. Are we going to communicate internally and externally that local-cli folder is "read-only" and no edits should be performed?

  2. How are we going to keep local-cli and react-native-local-cli in sync? Should maintainers of the latter keep an eye on the local-cli and make sure all changes (if any) are reapplied before a new version is cut? This is especially important in case of Metro team that might work on some files.

Action items:

Right now, the react-native-local-cli is set to alpha.4 version which is behind local-cli folder by a few commits (not really relevant to this PR). The last one is f8040ed - all newer than Nov, 5 are not present. See it here.

The changes are not really relevant to this PR (from the stability perspective). My idea is to merge this PR and once it happens, I will send another one the same day to bump react-native-local-cli with all the updates applied to local-cli right before this PR has been merged.

@grabbou grabbou mentioned this pull request Nov 18, 2018
5 tasks
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@axe-fb has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@@ -142,9 +142,6 @@
"build-ios-e2e": "detox build -c ios.sim.release",
"test-ios-e2e": "detox test -c ios.sim.release --cleanup"
},
"bin": {
Copy link
Member

Choose a reason for hiding this comment

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

Why don't we want this anymore? It seems like it is there to catch people from making mistakes. Is that mistake not possible anymore? Or we just think it isn't a big deal anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have kept this behaviour, but made react-native-local-cli to have "bin" property instead. So just declaring in a different npm module (since React Native itself doesn't have an executable anymore)

@react-native-bot
Copy link
Collaborator

@grabbou merged commit cb6eb03 into facebook:master.

@facebook facebook locked as resolved and limited conversation to collaborators Dec 3, 2018
@react-native-bot react-native-bot added the Merged This PR has been merged. label Dec 3, 2018
grabbou added a commit to react-native-community/cli that referenced this pull request Dec 3, 2018
Summary:
Continuation of facebook/react-native#22174 with an exception that `local-cli` folder is left in React Native repository to keep Facebook internal and React Native calls still working.

Separate strategy should be developed to remove all uses of `local-cli` in favor of dedicated utilities.
Pull Request resolved: facebook/react-native#22337

Reviewed By: TheSavior

Differential Revision: D13172898

Pulled By: cpojer

fbshipit-source-id: 0217867f9944648307475ebe629eb729da7bfaaf
@zpao zpao deleted the feat/extract-cli-new branch January 31, 2019 01:55
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants