-
Notifications
You must be signed in to change notification settings - Fork 24.4k
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
chore: rename normalize-color to normalize-colors (umbrella 480) #34571
Conversation
7f1417b
to
85f87b4
Compare
@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
1 similar comment
@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Base commit: 2d1d61a |
Base commit: 2d1d61a |
packages/normalize-colors/BUCK
Outdated
@@ -2,7 +2,7 @@ load("@fbsource//tools/build_defs/third_party:yarn_defs.bzl", "yarn_workspace") | |||
load("@fbsource//xplat/js:JS_DEFS.bzl", "rn_library") | |||
|
|||
rn_library( | |||
name = "normalize-color", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commenting here but valid for the whole PR:
Let's undo the BUCK update and the path change. It will make easier for us to import and merge this change 👍
I've also posted an update on the monorepo effort there:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done ✅
02bb773
to
4abf495
Compare
BUCK
Outdated
@@ -769,7 +769,7 @@ rn_library( | |||
"//xplat/js/RKJSModules/vendor/react-test-renderer:react-test-renderer", | |||
"//xplat/js/RKJSModules/vendor/scheduler:scheduler", | |||
"//xplat/js/react-native-github/packages/assets:assets", | |||
"//xplat/js/react-native-github/packages/normalize-color:normalize-color", | |||
"//xplat/js/react-native-github/packages/normalize-colors:normalize-color", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should also be reverted.
"description": "Color normalization for React Native.", | ||
"repository": { | ||
"type": "git", | ||
"url": "git@github.com:facebook/react-native.git", | ||
"directory": "packages/normalize-color" | ||
"directory": "packages/normalize-colors" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be reverted, alongside the folder renaming
@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
1 similar comment
@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
could you rebase this? I'll try to cary this out whenever I have some time. |
e874737
to
d5554bd
Compare
PR build artifact for d5554bd is ready. |
PR build artifact for d5554bd is ready. |
@hoxyq has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
…recated @react-native/normalize-color
PR build artifact for d7b39e7 is ready. |
PR build artifact for d7b39e7 is ready. |
@hoxyq has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
This pull request was successfully merged by @Titozzz in dc33559. When will my fix make it into a release? | Upcoming Releases |
Should we have also renamed |
We plan to do all renamings in Phase 4, as discussed in original RFC: react-native-community/discussions-and-proposals#480 |
…ebook#34571) Summary: ## Changelog <!-- Help reviewers and the release process by writing your own changelog entry. For an example, see: https://reactnative.dev/contributing/changelogs-in-pull-requests --> [General] [Changed] - Rename normalize-color to normalize-colors as part of react-native-community/discussions-and-proposals#480 Pull Request resolved: facebook#34571 Reviewed By: cortinico Differential Revision: D39235696 Pulled By: hoxyq fbshipit-source-id: b6d5fcae9fb5c953c2f7b48f73a95cd883ff8f63
Summary: To address the root cause of a recurring issue (#40797, #39692) where breaking changes to `react-native/normalize-colors` would be pulled into old versions of `deprecated-react-native-prop-types`, we recently change the dependency in the latter to use a semver range (facebook/react-native-deprecated-modules#27, #40869). For CI, we generally force `react-native/*` to be resolved only from Verdaccio locally published packages - ie, the current versions at source. The source version (currently `0.74.1`) isn't semver-compatible with `deprecated-react-native-prop-types`'s dependency (`^0.73.0`), so `npm install` was failing in CI with "no package found". We should be getting `0.73.2` from the public registry in this case. This restores a previous workaround added in #34571 but not updated since facebook/react-native-deprecated-modules#11 meant the dependency was now on the pluralised package. We have no dependency on the old non-plural package any more. ## Changelog: [INTERNAL] [FIXED] - CI/Verdaccio: Proxy `react-native/normalize-colors` from NPM for the `deprecated-react-native-prop-types` dependency. Pull Request resolved: #40971 Test Plan: CI Reviewed By: cipolleschi Differential Revision: D50298291 Pulled By: robhogan fbshipit-source-id: 4bf6503108335ffa52654346d1874c217071ff91
Summary
Changelog
[General] [Changed] - Rename normalize-color to normalize-colors as part of react-native-community/discussions-and-proposals#480
Test Plan