Fix release script: update spec/dummy lockfile before release-it#957
Fix release script: update spec/dummy lockfile before release-it#957
Conversation
The release script updated spec/dummy/Gemfile.lock AFTER release-it had already committed and pushed, causing the lockfile to be stale on main. This broke CI because bundler's frozen mode detected the gemspec version mismatch. Move spec/dummy bundle install to before release-it so the lockfile change is included in the release commit via release-it's git add. Also updates spec/dummy/Gemfile.lock from 9.6.0.rc.3 to 9.6.0 to fix the current CI failure on main. Co-Authored-By: Claude Opus 4.6 <[email protected]>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
WalkthroughThe release process was reorganized to update lockfiles earlier in the workflow. Documentation and the Rake task were modified to run yarn and npm installs as a pre-release step in the spec/dummy directory, removing subsequent commit and push logic that previously handled detected lockfile changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Greptile SummaryThis PR fixes a CI failure caused by a stale
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant R as release.rake
participant Root as Root Directory
participant Dummy as spec/dummy
participant RI as release-it
participant RG as RubyGems
Note over R: perform_release()
R->>Root: gem bump --no-commit
R->>Root: bundle install (updates root Gemfile.lock)
Note over Dummy: ✅ Moved here (was after release-it)
R->>Dummy: bundle install (updates spec/dummy/Gemfile.lock)
R->>RI: release-it (git add . → commits ALL changes incl. lockfiles, tags, pushes, npm publish)
R->>RG: gem release
Note over R: ❌ Removed: post-release bundle/npm install + follow-up commit
Last reviewed commit: 43194a9 |
| # Update spec/dummy lockfiles BEFORE release-it so they are included in the release commit. | ||
| # release-it does `git add .` which picks up all working tree changes. | ||
| spec_dummy_dir = File.join(release_root, "spec", "dummy") | ||
| Shakapacker::Utils::Misc.sh_in_dir(spec_dummy_dir, "bundle install") |
There was a problem hiding this comment.
npm install removed with no replacement
The old post-release block ran npm install in spec/dummy (to keep package-lock.json / yarn.lock in sync) before committing the lockfiles. The new code only runs bundle install — the npm lockfile step is gone entirely.
spec/dummy/package.json uses "shakapacker": "file:.yalc/shakapacker", so a plain npm install wouldn't wire up the freshly-released npm package anyway. But if the intent is to keep yarn.lock or package-lock.json current (e.g. for transitive dependency bumps between releases), those files will silently drift.
If the npm lockfiles are intentionally no longer managed during release, a comment explaining that would make the rationale clear. If they still need updating, a yarn install / npm install call should be added here alongside the bundle install, with the same placement before release-it.
|
Review: The fix is well-reasoned. Moving spec/dummy bundle install before release-it is the correct approach — it ensures the lockfile update is picked up by release-it's git add . and included in the release commit. Removes the post-release follow-up commit logic which was a workaround rather than a fix. Three notes: (1) npm install for spec/dummy is dropped entirely — since spec/dummy uses a yalc local link for shakapacker rather than a versioned npm reference, this is likely fine, but worth confirming the yarn.lock and package-lock.json in spec/dummy no longer need to be updated during releases. (2) The fix relies on release-it's default git add . behavior; no .release-it.json was found so defaults apply, but worth verifying git.addUntrackedFiles is not overridden. (3) The new bundle install runs unconditionally during dry runs too — consistent with root bundle install behavior, low concern. |
| # Update spec/dummy lockfiles BEFORE release-it so they are included in the release commit. | ||
| # release-it does `git add .` which picks up all working tree changes. | ||
| spec_dummy_dir = File.join(release_root, "spec", "dummy") | ||
| Shakapacker::Utils::Misc.sh_in_dir(spec_dummy_dir, "bundle install") |
There was a problem hiding this comment.
The old code also ran npm install in spec/dummy (updating yarn.lock / package-lock.json) — that step is dropped here. Since spec/dummy/package.json references shakapacker via "file:.yalc/shakapacker" rather than a semver range, the npm lockfiles don't pin the shakapacker version, so this is likely safe to omit. Just worth confirming intentionally.
Releasing ShakapackerThis guide is for Shakapacker maintainers who need to publish a new release. Prerequisites
Release Process1. Update the ChangelogAlways update CHANGELOG.md before running the release task. The release task reads the version from CHANGELOG.md and automatically creates a GitHub release from the changelog section.
If you forget this step, the release task will print a warning and the GitHub release will need to be created manually afterward using 2. Run the Release TaskThe simplest way to release is with no arguments — the task reads the version from CHANGELOG.md: # Recommended: reads version from CHANGELOG.md (requires step 1)
bundle exec rake create_release
# For a specific version (overrides CHANGELOG.md detection)
bundle exec rake "create_release[9.1.0]"
# For a beta release (note: use period, not dash)
bundle exec rake "create_release[9.2.0.beta.1]" # Creates npm package 9.2.0-beta.1
# For a release candidate
bundle exec rake "create_release[9.6.0.rc.0]"
# Dry run to test without publishing
bundle exec rake "create_release[9.1.0,true]"
# Override version policy checks (monotonic + changelog/bump consistency)
RELEASE_VERSION_POLICY_OVERRIDE=true bundle exec rake "create_release[9.1.0]"
bundle exec rake "create_release[9.1.0,false,true]"When called with no arguments,
Dry runs use a temporary git worktree so version bumps and installs do not modify your current checkout.
Use override only when needed:
3. What the Release Task DoesThe
4. Version FormatImportant: Use Ruby gem version format (no dashes):
The task automatically converts Ruby gem format to npm semver format:
CHANGELOG.md headers use npm semver format (with dashes):
Examples: # Regular release
bundle exec rake "create_release[9.1.0]" # Gem: 9.1.0, npm: 9.1.0
# Beta release
bundle exec rake "create_release[9.2.0.beta.1]" # Gem: 9.2.0.beta.1, npm: 9.2.0-beta.1
# Release candidate
bundle exec rake "create_release[10.0.0.rc.1]" # Gem: 10.0.0.rc.1, npm: 10.0.0-rc.1
# Prerelease: use /update-changelog rc first, then create_release reads it
bundle exec rake create_release # reads v10.0.0-rc.0 from CHANGELOG.md5. During the Release
6. After Release
Syncing GitHub Releases ManuallyIf the automatic GitHub release creation was skipped (e.g., CHANGELOG.md section was missing during release), you can create it manually after updating the changelog:
# Stable
bundle exec rake "sync_github_release[9.6.0]"
# Prerelease
bundle exec rake "sync_github_release[9.6.0.rc.1]"
TroubleshootingUncommitted Changes After ReleaseIf you see uncommitted changes to lockfiles after a release, this means:
Failed npm or RubyGems PublishIf publishing fails partway through:
GitHub Release Sync FailsIf package publishing succeeds but GitHub release creation fails:
Wrong Version FormatIf you accidentally use npm format (with dashes):
Manual Release StepsIf you need to release manually (not recommended):
Questions?If you encounter issues not covered here, please:
|
ReviewThe fix correctly identifies and addresses the root cause: spec/dummy lockfile updates were happening after Two issues worth addressing: 1. docs/releasing.md is now inaccurate The documentation still describes the old flow. Step 6 places lockfile updates after RubyGems publish, but in the updated code they now happen before 2. Progress logging removed without replacement The old code wrapped the spec/dummy install step with ZZZZ... banner separators, making it easy to spot in release logs. The new code runs the three install commands silently inline. A brief |
| - Runs `npm install` to update `package-lock.json` (yarn.lock may also be updated for multi-package-manager compatibility testing) | ||
| - Runs `yarn install` to refresh the Yarn-managed dummy app lockfile | ||
| - Runs `npm install` to keep `package-lock.json` in sync for npm compatibility/testing | ||
| 7. **Commits and pushes lockfile changes** automatically |
There was a problem hiding this comment.
This step no longer exists — the code that committed and pushed lockfile changes separately was removed in this PR. The lockfiles are now included in the release-it release commit via its git add .. This line should be removed (and step 8 renumbered to 7) to avoid misleading future maintainers.
| @@ -108,7 +108,8 @@ The `create_release` task automatically: | |||
| - Prompts for RubyGems OTP (2FA code) | |||
| 6. **Updates spec/dummy lockfiles:** | |||
There was a problem hiding this comment.
The ordering here is wrong after this fix. In the updated code, spec/dummy lockfile updates now happen before release-it runs (i.e., before step 4 "Publishes to npm"), not after RubyGems publish. This step should be moved up in the list to reflect the new sequence.
| Shakapacker::Utils::Misc.sh_in_dir(spec_dummy_dir, "bundle install") | ||
| Shakapacker::Utils::Misc.sh_in_dir(spec_dummy_dir, "yarn install") | ||
| Shakapacker::Utils::Misc.sh_in_dir(spec_dummy_dir, "npm install") |
There was a problem hiding this comment.
The old code had a ZZZZ... banner and "Updating spec/dummy dependencies" message before these steps, which made them easy to identify in release logs. Consider adding a puts here so maintainers can follow progress during a real release:
| Shakapacker::Utils::Misc.sh_in_dir(spec_dummy_dir, "bundle install") | |
| Shakapacker::Utils::Misc.sh_in_dir(spec_dummy_dir, "yarn install") | |
| Shakapacker::Utils::Misc.sh_in_dir(spec_dummy_dir, "npm install") | |
| puts "Updating spec/dummy lockfiles (will be included in the release commit)..." | |
| Shakapacker::Utils::Misc.sh_in_dir(spec_dummy_dir, "bundle install") | |
| Shakapacker::Utils::Misc.sh_in_dir(spec_dummy_dir, "yarn install") | |
| Shakapacker::Utils::Misc.sh_in_dir(spec_dummy_dir, "npm install") |
## Summary - **Fixes CI failure on main** caused by stale `spec/dummy/Gemfile.lock` (still referenced `9.6.0.rc.3` instead of `9.6.0`) - **Fixes the root cause in the release script**: moves `spec/dummy` `bundle install` to run **before** `release-it`, so the lockfile update is included in the release commit via `release-it`'s `git add .` ### What was happening The release flow was: 1. `gem bump` → bumps `version.rb` 2. `bundle install` → updates root `Gemfile.lock` 3. `release-it` → stages all changes, commits, tags, **pushes** 4. `bundle install` in `spec/dummy` → updates lockfile (too late — already pushed) 5. Commit + push lockfile changes (follow-up commit that never materialized) ### What this fixes Moves step 4 to between steps 2 and 3, so `release-it`'s `git add .` picks up the spec/dummy lockfile change in the same release commit. No more follow-up commit needed. ## Test plan - [ ] Verify CI passes on this PR (the `spec/dummy/Gemfile.lock` update fixes the immediate bundler frozen mode error) - [ ] On next release, verify `spec/dummy/Gemfile.lock` is included in the release commit 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Touches the automated release flow (ordering of versioning/commit steps), which could affect how release commits are created and published if the sequence is wrong, though the change is small and localized. > > **Overview** > Moves the `spec/dummy` `bundle install` step to run **before** `release-it` in `rakelib/release.rake`, ensuring dummy lockfile changes are picked up by `release-it`’s release commit and removing the post-release lockfile commit/push logic. > > Updates `spec/dummy/Gemfile.lock` to reference `shakapacker 9.6.0` instead of the prior `9.6.0.rc.3`. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 43194a9. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 4.6 <[email protected]>
## Summary - Added [Unreleased] changelog entries for two merged PRs missing from CHANGELOG.md: - **PR #963**: Fixed `Env#current` crashing when Rails is not loaded (bug fix by @ihabadham) - **PR #900**: Added Node package API documentation (by @justin808) - Skipped non-user-visible PRs: #958, #957, #959 ## Test plan - [x] `yarn lint` passes - [x] Changelog formatting matches existing conventions 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Documentation-only change confined to `CHANGELOG.md`, with no runtime or build behavior impact. > > **Overview** > Updates `CHANGELOG.md` under **[Unreleased]** to document two previously-merged changes: a fix preventing `Env#current` from crashing when Rails isn’t loaded, and new documentation for the Node package JavaScript API (`docs/node_package_api.md`). > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 81bf2b8. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 4.6 <[email protected]>
Summary
spec/dummy/Gemfile.lock(still referenced9.6.0.rc.3instead of9.6.0)spec/dummybundle installto run beforerelease-it, so the lockfile update is included in the release commit viarelease-it'sgit add .What was happening
The release flow was:
gem bump→ bumpsversion.rbbundle install→ updates rootGemfile.lockrelease-it→ stages all changes, commits, tags, pushesbundle installinspec/dummy→ updates lockfile (too late — already pushed)What this fixes
Moves step 4 to between steps 2 and 3, so
release-it'sgit add .picks up the spec/dummy lockfile change in the same release commit. No more follow-up commit needed.Test plan
spec/dummy/Gemfile.lockupdate fixes the immediate bundler frozen mode error)spec/dummy/Gemfile.lockis included in the release commit🤖 Generated with Claude Code
Note
Medium Risk
Touches the automated release flow (ordering of versioning/commit steps), which could affect how release commits are created and published if the sequence is wrong, though the change is small and localized.
Overview
Moves the
spec/dummybundle installstep to run beforerelease-itinrakelib/release.rake, ensuring dummy lockfile changes are picked up byrelease-it’s release commit and removing the post-release lockfile commit/push logic.Updates
spec/dummy/Gemfile.lockto referenceshakapacker 9.6.0instead of the prior9.6.0.rc.3.Written by Cursor Bugbot for commit 43194a9. Configure here.
Summary by CodeRabbit
Release Notes
Documentation
Chores