-
Notifications
You must be signed in to change notification settings - Fork 5
Use NX #117
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
Use NX #117
Conversation
|
6c41c3a to
a4c1455
Compare
|
I expect we can run all tests across all supported platforms in the near future 👍 |
9789a28 to
6217f14
Compare
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.
Pull Request Overview
This PR introduces the NX tool to orchestrate tasks across the mono-repo, streamlining build, test, and dependency management across multiple packages.
- Added NX project configuration files (project.json) for several packages.
- Updated package.json scripts to leverage NX commands.
- Adjusted CI workflow to execute tests via NX across multiple platforms.
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/node-addon-examples/project.json | Added NX targets for copying examples and converting gyp to CMake |
| packages/node-addon-examples/package.json | Modified test script to directly run verification prebuilds |
| packages/host/project.json | Introduced NX targets for building and generating weak node API |
| packages/host/package.json | Updated build-weak-node-api script to rely on NX target dependencies |
| packages/gyp-to-cmake/project.json | Added NX configuration for the build target |
| packages/ferric/project.json | Added NX configuration for the build target |
| packages/ferric/package.json | Bumped dependency version and added a build script |
| packages/ferric-example/project.json | Added NX configuration for building the ferric-example package |
| packages/cmake-rn/project.json | Added NX configuration with a dependency on react-native-node-api |
| packages/cmake-rn/package.json | Added react-native-node-api dependency |
| package.json | Replaced workspace test commands with NX command |
| nx.json | Introduced basic NX configuration |
| .github/workflows/check.yml | Updated CI workflow to run tests via NX and use a test matrix |
Comments suppressed due to low confidence (3)
.github/workflows/check.yml:18
- The node version 'lts/jod' may be a typo; please verify if this is the intended version, as it could affect CI consistency.
node-version: lts/jod
packages/host/package.json:46
- The removal of the 'npm run generate-weak-node-api' call suggests that the generation step is now handled by NX target dependencies; consider updating the documentation to clarify the new build workflow.
"build-weak-node-api": "cmake-rn --android --apple --no-auto-link --no-weak-node-api-linkage --xcframework-extension --source ./weak-node-api",
packages/node-addon-examples/package.json:14
- The revised test script now only calls 'verify-prebuilds.mts' without the previous 'copy-and-build' steps; please ensure that the new approach performs all necessary prebuild tasks as intended within the NX framework.
"test": "tsx scripts/verify-prebuilds.mts"
|
Got a little push back on this, which realized that I could probably do with less without taking on a new dependency: #120 |
This solves #51.
Merging this PR will: