-
Notifications
You must be signed in to change notification settings - Fork 198
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
Dev 325/setup nx release monorepo #2726
base: dev
Are you sure you want to change the base?
Conversation
|
WalkthroughThe changes introduce a new Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
Outside diff range and nitpick comments (6)
nx.json (2)
53-59
: LGTM: Comprehensive project inclusion with appropriate exclusionThe
projects
array effectively includes all packages, SDKs, and specific services while excluding the "private" project. This setup ensures that all relevant projects are included in the release process while preventing unintended releases of private code.For improved clarity, consider adding a comment explaining the purpose of the
!private
entry:"projects": [ "packages/*", "sdks/*", "services/websocket-service", "services/workflows-service", + // Exclude the private project from releases "!private" ],
63-76
: LGTM: Comprehensive changelog configuration with GitHub integrationThe
changelog
configuration is well-structured, covering both project-level and workspace-level changelog generation. Enabling GitHub releases for both is excellent for visibility and tracking. The rendering options for project changelogs will provide rich, informative changelogs.For consistency, consider adding the same rendering options to the workspace changelog:
"workspaceChangelog": { + "renderOptions": { + "authors": true, + "mapAuthorsToGitHubUsernames": true, + "commitReferences": true, + "versionTitleDate": true + }, "createRelease": "github" }This will ensure that the workspace changelog has the same level of detail as the project changelogs.
.github/workflows/test-publish-package.yml (4)
1-15
: LGTM! Consider adding a description to the workflow.The workflow name, triggers, and concurrency settings are well-defined. The flexibility to run on
dev
branch pushes or as a reusable workflow is a good approach.Consider adding a
description
field to provide more context about the workflow's purpose. For example:name: Under Testing - Publish-packages description: Tests and publishes packages when changes are pushed to the dev branch
20-64
: LGTM! Consider using a more specific Node.js version.The
packages
job is well-structured with proper environment setup and dependency caching. The use ofpnpm nx release --skip-publish
is appropriate for this stage.Consider using a more specific Node.js version instead of
18.12.1
. You could use the latest LTS version or pin to a specific minor version for better consistency:- name: Install Node.js uses: actions/setup-node@v3 with: node-version: 18.x # Latest 18.x LTS versionTools
actionlint
47-47: shellcheck reported issue in this script: SC2086:info:1:41: Double quote to prevent globbing and word splitting
(shellcheck)
106-111
: Consider removing the commented-out section if it's no longer needed.There's a commented-out section that seems to be a more specific publish command. If this is no longer needed, it would be best to remove it to keep the workflow file clean.
If you decide to keep it for reference, consider adding a comment explaining why it's kept and when it might be used.
44-48
: Consider addressing shellcheck warnings for improved script robustness.There are shellcheck warnings about potential globbing and word splitting in the "Get pnpm store directory" steps. While not critical, addressing these would improve the script's robustness.
Apply this change to both occurrences of the step:
- name: Get pnpm store directory id: pnpm-cache shell: bash run: | - echo "STORE_PATH=$(pnpm store path)" >> $GITHUB_OUTPUT + echo "STORE_PATH=$(pnpm store path)" >> "$GITHUB_OUTPUT"Also applies to: 90-94
Tools
actionlint
47-47: shellcheck reported issue in this script: SC2086:info:1:41: Double quote to prevent globbing and word splitting
(shellcheck)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- .github/workflows/test-publish-package.yml (1 hunks)
- nx.json (1 hunks)
- package.json (1 hunks)
Additional context used
actionlint
.github/workflows/test-publish-package.yml
47-47: shellcheck reported issue in this script: SC2086:info:1:41: Double quote to prevent globbing and word splitting
(shellcheck)
93-93: shellcheck reported issue in this script: SC2086:info:1:41: Double quote to prevent globbing and word splitting
(shellcheck)
Additional comments not posted (7)
nx.json (3)
38-39
: LGTM: Appropriate release configuration for independent projectsThe
projectsRelationship
set to "independent" allows for separate versioning of projects in the monorepo, which is suitable for a release monorepo setup. ThereleaseTagPattern
will create unique tags for each project release, following the format{projectName}@{version}
. This configuration aligns well with the PR objective.
60-62
: LGTM: Conventional commits enabled for versioningEnabling
conventionalCommits
for versioning is a good practice. It ensures that the version bumps are determined based on the commit messages, which is consistent with the earlierconventionalCommits
configuration.
37-77
: Overall: Well-structured release configuration for a monorepoThe changes to
nx.json
effectively set up a release process for a monorepo, aligning well with the PR objective. The configuration covers independent project releases, conventional commits, comprehensive project inclusion, and detailed changelog generation with GitHub integration.A few minor suggestions have been made to enhance the configuration:
- Adjusting the semantic versioning bumps for different commit types.
- Adding a clarifying comment for the private project exclusion.
- Extending the rendering options to the workspace changelog for consistency.
These changes will further improve the release process and maintainability of the configuration.
.github/workflows/test-publish-package.yml (2)
18-19
: LGTM! Good use of workflow reusability.Using a separate CI workflow promotes modularity and reusability in your GitHub Actions setup.
66-105
: LGTM! Well-structured publish job.The
publish_packages
job is well-configured with appropriate conditions and environment setup. The use ofpnpm nx release publish
with the explicit NPM registry is good for clarity.Also applies to: 113-118
Tools
actionlint
93-93: shellcheck reported issue in this script: SC2086:info:1:41: Double quote to prevent globbing and word splitting
(shellcheck)
package.json (2)
Line range hint
1-93
: Summary: Nx tooling upgrade implemented as intendedThe changes in this
package.json
file align with the PR objective of setting up an Nx release monorepo. The main modifications involve upgrading the Nx tooling, which is a significant change that may affect the project's build and development processes.Recommendations:
- Document this upgrade in the project's changelog or release notes.
- Consider updating the
engines
section to specify any new Node.js version requirements that may have been introduced by this Nx upgrade.- Review and update the project's documentation, especially any sections related to development setup or contribution guidelines, to reflect the new Nx version.
86-87
: Significant Nx upgrade detected. Ensure compatibility and update related configurations.The changes introduce a major upgrade to the Nx tooling:
- Added
@nx/js
package with version "^19.5.7"- Updated
nx
package from "15.0.2" to "^19.5.7"This represents a significant version jump from 15.x to 19.x, which likely includes breaking changes and new features. Please ensure the following:
- Review the Nx upgrade guide for any breaking changes between version 15 and 19.
- Update any Nx configuration files (e.g.,
nx.json
,workspace.json
) to align with the new version requirements.- Test all Nx commands and scripts to verify they still work as expected.
- Update your CI/CD pipelines if they depend on specific Nx behaviors that might have changed.
To help verify the impact of this upgrade, you can run the following commands:
This script will help identify any Nx-related configurations and scripts that might need attention due to the upgrade.
…mentioned in release
Summary by CodeRabbit
New Features
release
section in project configuration for independent project releases and semantic versioning.Bug Fixes
Documentation