Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Local cli/android/normalize project name #18275

Conversation

Titozzz
Copy link
Collaborator

@Titozzz Titozzz commented Mar 8, 2018

Motivation

Scoped packages are starting to be the new thing, and gradle does not work properly with '/' in the project name, so this PR links them and replaces '/' by '_' . This only affects android.

Test Plan

I added tests in the 2 impacted functions + a test file for the normalizer function

Related PRs

#17000 This was the initial PR but this is done without mutations

Release Notes

[CLI] [BUGFIX] [local-cli/link/link.js] - On android, Scoped packages will now get the '/' replaced with '_' to ensure gradle works nicely. ⚠️ However if you previously linked scoped packages, they will get linked again. ⚠️

@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 Mar 8, 2018
@pull-bot
Copy link

pull-bot commented Mar 8, 2018

Warnings
⚠️

📋 Release Notes - This PR may have incorrectly formatted Release Notes.

Generated by 🚫 dangerJS

@Titozzz
Copy link
Collaborator Author

Titozzz commented Mar 15, 2018

@Kureev

@Kureev
Copy link
Contributor

Kureev commented Mar 16, 2018

Hey!

Awesome PR, and definitely important for Android developers. I have only one thing to request: can you please verify that unlink also works as expected? 🙌

@Titozzz
Copy link
Collaborator Author

Titozzz commented Mar 16, 2018

Hello, I did add some test to the helpers used by both link and unlink:

makeBuildPatch.js
makeSettingsPatch.js

so everything should be fine

@Kureev
Copy link
Contributor

Kureev commented Mar 16, 2018

@facebook-github-bot shipit

@facebook-github-bot
Copy link
Contributor

Something went wrong executing that command, @hramos could you take a look?

@facebook-github-bot facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Mar 16, 2018
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.

@Kureev is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

hamaron pushed a commit to hamaron/react-native that referenced this pull request Apr 9, 2018
Summary:
<!--
Thank you for sending the PR! We appreciate you spending the time to work on these changes.

Help us understand your motivation by explaining why you decided to make this change.

You can learn more about contributing to React Native here: http://facebook.github.io/react-native/docs/contributing.html

Happy contributing!

-->

Scoped packages are starting to be the new thing, and gradle does not work properly with '/' in the project name, so this PR links them and replaces '/' by '_' . This only affects android.

I added tests in the 2 impacted functions + a test file for the normalizer function

<!--
Help reviewers and the release process by writing your own release notes

**INTERNAL and MINOR tagged notes will not be included in the next version's final release notes.**

-->
[CLI] [BUGFIX] [local-cli/link/link.js] - On android, Scoped packages will now get the '/' replaced with '_' to ensure gradle works nicely.  ⚠️ However if you previously linked scoped packages, they will get linked again. ⚠️
Closes facebook#18275

Differential Revision: D7305227

Pulled By: hramos

fbshipit-source-id: 1c95563e884175529692948b29407a7733c44353
campsafari pushed a commit to exozet/react-native that referenced this pull request Apr 11, 2018
Summary:
<!--
Thank you for sending the PR! We appreciate you spending the time to work on these changes.

Help us understand your motivation by explaining why you decided to make this change.

You can learn more about contributing to React Native here: http://facebook.github.io/react-native/docs/contributing.html

Happy contributing!

-->

Scoped packages are starting to be the new thing, and gradle does not work properly with '/' in the project name, so this PR links them and replaces '/' by '_' . This only affects android.

I added tests in the 2 impacted functions + a test file for the normalizer function

<!--
Help reviewers and the release process by writing your own release notes

**INTERNAL and MINOR tagged notes will not be included in the next version's final release notes.**

-->
[CLI] [BUGFIX] [local-cli/link/link.js] - On android, Scoped packages will now get the '/' replaced with '_' to ensure gradle works nicely.  ⚠️ However if you previously linked scoped packages, they will get linked again. ⚠️
Closes facebook#18275

Differential Revision: D7305227

Pulled By: hramos

fbshipit-source-id: 1c95563e884175529692948b29407a7733c44353
LukeDurrant pushed a commit to LukeDurrant/react-native that referenced this pull request Apr 11, 2018
Summary:
<!--
Thank you for sending the PR! We appreciate you spending the time to work on these changes.

Help us understand your motivation by explaining why you decided to make this change.

You can learn more about contributing to React Native here: http://facebook.github.io/react-native/docs/contributing.html

Happy contributing!

-->

Scoped packages are starting to be the new thing, and gradle does not work properly with '/' in the project name, so this PR links them and replaces '/' by '_' . This only affects android.

I added tests in the 2 impacted functions + a test file for the normalizer function

<!--
Help reviewers and the release process by writing your own release notes

**INTERNAL and MINOR tagged notes will not be included in the next version's final release notes.**

-->
[CLI] [BUGFIX] [local-cli/link/link.js] - On android, Scoped packages will now get the '/' replaced with '_' to ensure gradle works nicely.  ⚠️ However if you previously linked scoped packages, they will get linked again. ⚠️
Closes facebook#18275

Differential Revision: D7305227

Pulled By: hramos

fbshipit-source-id: 1c95563e884175529692948b29407a7733c44353
LukeDurrant pushed a commit to LukeDurrant/react-native that referenced this pull request Apr 11, 2018
Summary:
<!--
Thank you for sending the PR! We appreciate you spending the time to work on these changes.

Help us understand your motivation by explaining why you decided to make this change.

You can learn more about contributing to React Native here: http://facebook.github.io/react-native/docs/contributing.html

Happy contributing!

-->

Scoped packages are starting to be the new thing, and gradle does not work properly with '/' in the project name, so this PR links them and replaces '/' by '_' . This only affects android.

I added tests in the 2 impacted functions + a test file for the normalizer function

<!--
Help reviewers and the release process by writing your own release notes

**INTERNAL and MINOR tagged notes will not be included in the next version's final release notes.**

-->
[CLI] [BUGFIX] [local-cli/link/link.js] - On android, Scoped packages will now get the '/' replaced with '_' to ensure gradle works nicely.  ⚠️ However if you previously linked scoped packages, they will get linked again. ⚠️
Closes facebook#18275

Differential Revision: D7305227

Pulled By: hramos

fbshipit-source-id: 1c95563e884175529692948b29407a7733c44353
bunnyc1986 pushed a commit to bunnyc1986/react-native that referenced this pull request May 11, 2018
Summary:
<!--
Thank you for sending the PR! We appreciate you spending the time to work on these changes.

Help us understand your motivation by explaining why you decided to make this change.

You can learn more about contributing to React Native here: http://facebook.github.io/react-native/docs/contributing.html

Happy contributing!

-->

Scoped packages are starting to be the new thing, and gradle does not work properly with '/' in the project name, so this PR links them and replaces '/' by '_' . This only affects android.

I added tests in the 2 impacted functions + a test file for the normalizer function

<!--
Help reviewers and the release process by writing your own release notes

**INTERNAL and MINOR tagged notes will not be included in the next version's final release notes.**

-->
[CLI] [BUGFIX] [local-cli/link/link.js] - On android, Scoped packages will now get the '/' replaced with '_' to ensure gradle works nicely.  ⚠️ However if you previously linked scoped packages, they will get linked again. ⚠️
Closes facebook#18275

Differential Revision: D7305227

Pulled By: hramos

fbshipit-source-id: 1c95563e884175529692948b29407a7733c44353
@petermekhaeil
Copy link

Is this PR planned for a release?

@hramos
Copy link
Contributor

hramos commented Jun 13, 2018

@petermekhaeil it's in 0.56.0-rc. You can corroborate this by looking at the tags in dbd4759. It's also called out in the CHANGELOG.