-
Notifications
You must be signed in to change notification settings - Fork 93
[RHCLOUD-42870] Nx tool migration #1518
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
base: master
Are you sure you want to change the base?
Conversation
|
@Jakub007d is attempting to deploy a commit to the data-driven-forms Team on Vercel. A member of the Team first needs to authorize it. |
3707bc0 to
1d2408e
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1518 +/- ##
=======================================
Coverage 94.27% 94.27%
=======================================
Files 185 185
Lines 3389 3389
Branches 1462 1462
=======================================
Hits 3195 3195
Misses 194 194 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
caa6f4a to
2c284a9
Compare
de61b39 to
a484371
Compare
a484371 to
23e49e8
Compare
package.json
Outdated
| "@khala/npm-release-monorepo": "^2.5.2", | ||
| "@khala/wildcard-release-notes": "^2.5.2", | ||
| "@nx/js": "22.0.1", | ||
| "@semantic-release/exec": "^6.0.3", |
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.
@Jakub007d since we are switching to NX's release process, I think we can remove the legacy semantic-release packages right?
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 are right removed
.github/workflows/main.yml
Outdated
| run: npx nx release --first-release --dry-run --verbose | ||
|
|
||
| # Job that runs after merge (on push to master) | ||
| post-merge: |
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.
@Jakub007d I'm wondering if we want the PR checks to also run for merge jobs, and just exclude the release for PRs. Something like we do here: https://github.com/RedHatInsights/javascript-clients/blob/main/.github/workflows/ci.yml
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.
Changed
package.json
Outdated
| "lerna": "^4.0.0", | ||
| "ncp": "^2.0.0", | ||
| "sass": "^1.77.8", | ||
| "nx": "^22.0.1", |
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.
NIT - looks like in the past 2 days NX released 22.0.2 😂
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.
Oh :D updated
|
@Jakub007d since we are moving to NX, should we remove Lerna as well and replace with NX? https://github.com/data-driven-forms/react-forms/blob/master/lerna.json |
| "fallbackCurrentVersionResolver": "disk" | ||
| } | ||
| }, | ||
| "namedInputs": { |
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.
@Jakub007d quick AI review comment:
Your nx.json sets namedInputs.default to only sharedGlobals. For run-commands with cache=true this can cause stale cache. Suggest:
In nx.json:
Set namedInputs.default to include project sources, e.g. ["sharedGlobals", "{projectRoot}/**/*", "!{projectRoot}/dist/**", "!{projectRoot}/.next/**"].
Optionally add a production namedInput variant if needed.
Or, per target in project.json, define inputs: ["default", "^default"].
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.
fixed
5f2166a to
06e8e80
Compare
|
Hmm the tests for the react-form-renderer started failing im looking into it |
707446d to
3cfa84f
Compare
|
Fixed the failing test |
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.
Running npx nx graph fails with a MultipleProjectsWithSameNameError. This is because many sub-projects (e.g., .../ant-component-mapper/checkbox and .../mui-component-mapper/checkbox) are being detected with the same name ("checkbox").
This error breaks the Nx dependency graph, which means core features like nx affected will not work.
Fixes #(issue) (if applicable)
Description
As a part of the RHCLOUD-42870 task i have added the nx tool to help with running tests/linters/builds/type-checks/releases/publish in CI. Release and publish would be ran after the PR gets merged (For proper release/publish we need to add 2 secrets to the repository (NOT TESTED only the dry run was ran so far)).
Please include a summary of the change.
Schema (if applicable)
Checklist: (please see documentation page for more information)
Yarn buildpassesYarn lintpassesYarn testpassesfix|feat({scope}): {description}fix(pf3): wizard correctly handles next buttonFix button on documenation example page