-
Notifications
You must be signed in to change notification settings - Fork 24.3k
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
Add bots as a yarn workspace and update danger action #34652
Conversation
Base commit: 4e70376 |
Base commit: 4e70376 |
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.
You should also now be able to remove the "yarn.lock" in the bots folder.
@lunaleaps has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
1 similar comment
@lunaleaps has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Summary: When working on facebook#34614, danger is failing because it doesn't share `node_modules` with the root directory where `typescript` is installed as we added it as a parser in our eslint config. By setting `bots` as a yarn workspace, dependencies are all installed under the root `node_modules` folder and in local testing (detailed in test section) we no longer have the `typescript module not found` error. However, danger will continue to fail on facebook#34614 as the `danger_pr` Github action runs from what's defined on `main`. Once these changes land, I can rebase facebook#34614 on it and danger's eslint should pass. ## 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 --> [Internal][Fixed] - Add `bots` directory as a yarn workspace and update `danger_pr` Github action Pull Request resolved: facebook#34652 Test Plan: To verify this fix I had to run: ``` react-native $ yarn && cd bots react-native/bots$ yarn run danger pr facebook#34614 ``` which resulted in ``` ❯ yarn run danger pr facebook#34614 yarn run v1.22.19 $ lunaleaps/react-native/node_modules/.bin/danger pr facebook#34614 Starting Danger PR on facebook#34614 Danger: ✓ found only warnings, not failing the build ## Warnings :lock: package.json - <i>Changes were made to package.json. This will require a manual import by a Facebook employee.</i> ✨ Done in 12.78s. ``` Verified this also on another PR: ``` yarn run danger pr facebook#34650 ``` Reviewed By: NickGerleman Differential Revision: D39435286 Pulled By: lunaleaps fbshipit-source-id: b560d92635be8d95baf27274f745315aaf56d748
1f3088c
to
64d6d75
Compare
This pull request was exported from Phabricator. Differential Revision: D39435286 |
This pull request was successfully merged by @lunaleaps in 767f8e0. When will my fix make it into a release? | Upcoming Releases |
Summary: allow-large-files When working on facebook#34614, danger is failing because it doesn't share `node_modules` with the root directory where `typescript` is installed as we added it as a parser in our eslint config. By setting `bots` as a yarn workspace, dependencies are all installed under the root `node_modules` folder and in local testing (detailed in test section) we no longer have the `typescript module not found` error. However, danger will continue to fail on facebook#34614 as the `danger_pr` Github action runs from what's defined on `main`. Once these changes land, I can rebase facebook#34614 on it and danger's eslint should pass. ## 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 --> [Internal][Fixed] - Add `bots` directory as a yarn workspace and update `danger_pr` Github action Pull Request resolved: facebook#34652 Test Plan: To verify this fix I had to run: ``` react-native $ yarn && cd bots react-native/bots$ yarn run danger pr facebook#34614 ``` which resulted in ``` ❯ yarn run danger pr facebook#34614 yarn run v1.22.19 $ lunaleaps/react-native/node_modules/.bin/danger pr facebook#34614 Starting Danger PR on facebook#34614 Danger: ✓ found only warnings, not failing the build ## Warnings :lock: package.json - <i>Changes were made to package.json. This will require a manual import by a Facebook employee.</i> ✨ Done in 12.78s. ``` Verified this also on another PR: ``` yarn run danger pr facebook#34650 ``` Reviewed By: NickGerleman Differential Revision: D39435286 Pulled By: lunaleaps fbshipit-source-id: 8c82f49facf162f4fc0918e3abd95eb7e4ad1e37
Summary
When working on #34614, danger is failing because it doesn't share
node_modules
with the root directory wheretypescript
is installed as we added it as a parser in our eslint config.By setting
bots
as a yarn workspace, dependencies are all installed under the rootnode_modules
folder and in local testing (detailed in test section) we no longer have thetypescript module not found
error. However, danger will continue to fail on #34614 as thedanger_pr
Github action runs from what's defined onmain
.Once these changes land, I can rebase #34614 on it and danger's eslint should pass.
Changelog
[Internal][Fixed] - Add
bots
directory as a yarn workspace and updatedanger_pr
Github actionTest Plan
To verify this fix I had to run:
which resulted in
Verified this also on another PR: