-
Notifications
You must be signed in to change notification settings - Fork 3
chore: add-patch-release-docs-and-workflow #300
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: main
Are you sure you want to change the base?
Conversation
|
View your CI Pipeline Execution ↗ for commit 73a2622
☁️ Nx Cloud last updated this comment at |
678b7bb
to
3d4878e
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (18.75%) is below the target coverage (40.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #300 +/- ##
=======================================
Coverage 18.75% 18.75%
=======================================
Files 138 138
Lines 27368 27368
Branches 951 951
=======================================
Hits 5132 5132
Misses 22236 22236 🚀 New features to boost your workflow:
|
3d4878e
to
73a2622
Compare
WalkthroughUpdates CI workflow to replace nx-set-shas with a custom SHA resolution step and switch docs deployment to GitHub Pages. Adds a new manually triggered patch release workflow handling versioning, build/test, publish, tagging, and docs deployment. Updates release documentation with a patch release process and instructions. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant GH as GitHub Actions (CI)
participant gh as gh CLI
participant Nx as Nx Affected
participant Pages as GitHub Pages
Dev->>GH: Push/PR to repository
GH->>gh: Resolve latest release tag
gh-->>GH: BASE_SHA
GH->>GH: Set NX_BASE/NX_HEAD from SHAs
GH->>Nx: Run affected build/test using NX_BASE/NX_HEAD
GH->>Pages: Deploy docs via JamesIves action
Note over GH,Pages: Docs deployed from ./docs
sequenceDiagram
autonumber
actor Dev as Developer
participant GH as Workflow: Release Patch
participant PNPM as pnpm / Node
participant Nx as Nx / Nx Cloud
participant CS as changesets
participant NPM as npm Registry
participant Git as Git Tags
participant Pages as GitHub Pages
Dev->>GH: workflow_dispatch (branch-name)
GH->>Git: Checkout branch-name
GH->>PNPM: Setup Node 22.14 + pnpm, install deps
GH->>Nx: Enable Nx Cloud, compute affected (nx-set-shas)
GH->>Nx: Build & test affected
GH->>CS: Version packages
GH->>NPM: Publish with tokens
GH->>Git: Create & push tags
GH->>Pages: Deploy API docs (JamesIves)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
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: 2
🧹 Nitpick comments (8)
.github/workflows/ci.yml (1)
70-72
: Improve error message when no release exists.When
LATEST_RELEASE_TAG
is empty, the script exits with code 1 but provides no error message. This will make debugging CI failures harder.Apply this diff to add a descriptive error message:
if [ -z "$LATEST_RELEASE_TAG" ]; then + echo "Error: No GitHub release found. Cannot determine base SHA." >&2 exit 1 else
contributing_docs/releases.md (2)
117-117
: Clarify the version alignment scenario.The statement "we could be on 1.0.1 on
npm
and ourmain
branch may be on versions1.0.0
" is confusing. This seems backward—typically main would be ahead of npm, not behind.Consider revising to clarify the intended scenario, or provide a concrete example of when this situation would occur.
126-136
: Add concrete git commands for the cherry-pick process.The patch release steps would be clearer with actual git commands. Consider adding examples like:
# Create patch branch from release tag git checkout -b patch-release-1.0.1 v1.0.0 # Cherry-pick the fix commit git cherry-pick <commit-sha> # Add a changeset pnpm changeset # Push the branch git push origin patch-release-1.0.1This would make the process more actionable for developers following the documentation.
.github/workflows/patch-release.yml (5)
14-14
: Inconsistent NPM token naming between env and step.
NPM_ACCESS_TOKEN
is defined at the workflow level (line 14) but the publish step also setsNPM_TOKEN
(line 73) with the same value. The.npmrc
file (line 69) uses_authToken=$NPM_ACCESS_TOKEN
, soNPM_TOKEN
appears unused.Consider removing the redundant
NPM_TOKEN
environment variable from the publish step:- name: Publish patch run: | echo "//registry.npmjs.org/:_authToken=$NPM_ACCESS_TOKEN" > .npmrc pnpm publish -r env: NPM_CONFIG_PROVENANCE: 'true' - NPM_TOKEN: ${{ secrets.NPM_ACCESS_TOKEN }}
Also applies to: 67-74
33-35
: Add error handling for non-existent branch.The branch checkout will fail with a generic git error if the branch doesn't exist. Consider adding validation:
- name: Checkout patch branch run: | + if ! git show-ref --verify --quiet refs/remotes/origin/${{ github.event.inputs.branch-name }}; then + echo "Error: Branch '${{ github.event.inputs.branch-name }}' does not exist" >&2 + exit 1 + fi git checkout ${{ github.event.inputs.branch-name }}
41-44
: Inconsistent Node.js version specification with CI workflow.This workflow hardcodes Node.js version '22.14.0', while ci.yml uses
node-version-file: '.node-version'
. This creates a maintenance burden and potential version drift.Apply this diff to align with the CI workflow:
- uses: actions/setup-node@v4 with: - node-version: '22.14.0' + node-version-file: '.node-version' cache: 'pnpm'
58-70
: Add verification between versioning and publishing.The workflow versions packages (line 59) and then directly publishes (line 70) without verifying that versioning succeeded or that there are actual changes to publish. If
changeset version
finds no changesets, publishing will still attempt to run.Consider adding a verification step:
- name: Version packages run: pnpm exec changeset version env: GITHUB_TOKEN: ${{ secrets.GH_TOKEN }} + - name: Check for version changes + id: version-check + run: | + if git diff --quiet; then + echo "No version changes detected. Ensure changesets exist on this branch." + exit 1 + fi + # Build and test affected packages - name: Build and test run: pnpm exec nx affected -t build lint test e2e-ci
67-81
: Consider tagging before publishing for safer rollback.Currently, the workflow publishes to npm (lines 67-74) and then creates git tags (lines 76-81). If tag creation fails, packages will be published to npm without corresponding git tags, making it difficult to track what was released.
Consider reordering so tags are created before publishing. If tag creation fails, nothing has been published yet. If publishing fails after tagging, the tags can be deleted.
+ # Use changeset tag to create git tags according to changesets config + - name: Create git tags + run: | + git config --global user.email "[email protected]" + git config --global user.name "GitHub Actions" + pnpm exec changeset tag + - name: Publish patch run: | echo "//registry.npmjs.org/:_authToken=$NPM_ACCESS_TOKEN" > .npmrc pnpm publish -r env: NPM_CONFIG_PROVENANCE: 'true' - NPM_TOKEN: ${{ secrets.NPM_ACCESS_TOKEN }} - # Use changeset tag to create git tags according to changesets config - - name: Create and push git tags + - name: Push git tags run: | - git config --global user.email "[email protected]" - git config --global user.name "GitHub Actions" - pnpm exec changeset tag git push --follow-tagsHowever, note that changesets may expect packages to be published before tagging. Verify this doesn't break the changeset workflow.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/ci.yml
(2 hunks).github/workflows/patch-release.yml
(1 hunks)contributing_docs/releases.md
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: pr
🔇 Additional comments (3)
contributing_docs/releases.md (1)
143-148
: LGTM!The benefits section clearly articulates the advantages of the trunk-based patch release approach.
.github/workflows/patch-release.yml (2)
3-8
: LGTM on workflow trigger configuration.The workflow_dispatch trigger with required branch-name input is correctly configured. The description clearly indicates the expected format.
53-54
: Ensure consistent SHA resolution in patch-release workflow.This job uses
nrwl/nx-set-shas@v4
, butci.yml
relies on a customgh release view
script to compute the base SHA for patch branches. Confirm thatnx-set-shas
correctly identifies the parent-release SHA on patch branches; if not, switch this step to the same GitHub CLI logic to prevent mismatched SHAs.
- name: Publish api docs | ||
uses: JamesIves/github-pages[email protected] | ||
with: | ||
source-dir: docs | ||
pr-comment: 'none' |
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.
🧩 Analysis chain
Verify the correct parameter name for JamesIves/github-pages-deploy-action.
The action configuration uses source-dir
, but the JamesIves/github-pages-deploy-action typically uses folder
as the parameter name for specifying the deployment directory.
🌐 Web query:
What is the correct parameter name for specifying the source directory in JamesIves/github-pages-deploy-action version 4.7.3?
Error during web search
🌐 Web query:
JamesIves/github-pages-deploy-action source-dir folder input
💡 Result:
The action expects the input name folder (not source-dir). Set with:
-
folder: build (or . for repo root; you can use ~ to prefix an absolute path). Example:
-
name: Deploy
uses: JamesIves/github-pages-deploy-action@v4
with:
folder: build
References: [1][2]
Sources
[1] JamesIves github-pages-deploy-action examples/docs. [2] README/packaged docs and examples showing folder
is the required input.
Fix the deployment input key
In .github/workflows/ci.yml, change the input from source-dir
to folder
:
- with:
- source-dir: docs
+ with:
+ folder: docs
pr-comment: 'none'
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
- name: Publish api docs | |
uses: JamesIves/[email protected] | |
with: | |
source-dir: docs | |
pr-comment: 'none' | |
- name: Publish api docs | |
uses: JamesIves/[email protected] | |
with: | |
folder: docs | |
pr-comment: 'none' |
🤖 Prompt for AI Agents
In .github/workflows/ci.yml around lines 124 to 128, the GitHub Pages Deploy
action is using the deprecated input key `source-dir`; replace that key with
`folder` and set its value to the docs directory (i.e. change `source-dir: docs`
to `folder: docs`) so the JamesIves/[email protected] receives
the correct input.
- name: Publish api docs | ||
uses: JamesIves/[email protected] | ||
with: | ||
folder: docs | ||
commit-message: 'chore: release-api-docs-patch' |
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.
🛠️ Refactor suggestion | 🟠 Major
Parameter name inconsistency with CI workflow.
This workflow uses folder: docs
(line 90) while ci.yml uses source-dir: docs
(line 127) for the same JamesIves/github-pages-deploy-action. They should use the same parameter name.
Based on the JamesIves/github-pages-deploy-action documentation, folder
is the correct parameter name. The ci.yml workflow should be updated to match.
Note: This issue was also flagged in the ci.yml review above.
🤖 Prompt for AI Agents
In .github/workflows/patch-release.yml around lines 87 to 91 there is a
parameter inconsistency with another workflow (ci.yml): this job uses "folder:
docs" while ci.yml uses "source-dir: docs"; update the ci.yml workflow to use
the correct parameter name by replacing "source-dir: docs" with "folder: docs"
(and confirm both workflows use the same action version and parameter name so
they match).
JIRA Ticket
TBD
Description
I wanted to have an automated way for us to release patches. Because we merge to main, the cleanest way, as per trunk based development is to do the following:
the patch release should have a changeset (probably with the cherry-picked commit)
This way we A) have a standard process to release and B) release with provenance even for bug patches.
it's not required, but thought it would be helpful to have.
https://trunkbaseddevelopment.com/branch-for-release/#fix-production-bugs-on-trunk feel free to read more here
Summary by CodeRabbit
Chores
Documentation