Add final summary output to rake release#1041
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds release tracking and summary to the rake release flow: collects staged files, captures released gem and npm versions, extracts the relevant CHANGELOG.md section, passes it to GitHub sync, and prints a consolidated post-run summary that distinguishes dry-run from live runs. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
rakelib/release.rake (1)
656-662: Please add coverage for the new release summary flow.This task wiring changed observable behavior (Line 661) and depends on the new return payload shape from
perform_release; add tests for dry-run and live summary branches to guard against regressions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rakelib/release.rake` around lines 656 - 662, Add unit tests that cover the new release summary flow by asserting the return payload shape from perform_release and the behavior of print_release_summary for both dry-run and live releases: mock or stub perform_release to return the new payload structure and verify print_release_summary is called with the expected release_result when is_dry_run=true (dry-run branch) and when is_dry_run=false (live branch), and include assertions for any added fields on release_result that the rake task now relies on to prevent regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@rakelib/release.rake`:
- Around line 656-662: Add unit tests that cover the new release summary flow by
asserting the return payload shape from perform_release and the behavior of
print_release_summary for both dry-run and live releases: mock or stub
perform_release to return the new payload structure and verify
print_release_summary is called with the expected release_result when
is_dry_run=true (dry-run branch) and when is_dry_run=false (live branch), and
include assertions for any added fields on release_result that the rake task now
relies on to prevent regressions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 74f6e2ba-8c60-499c-a292-480ceea3900f
📒 Files selected for processing (1)
rakelib/release.rake
Greptile SummaryThis PR adds a final summary output to Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Rake as rake :release
participant PR as perform_release
participant WRC as with_release_checkout
participant SGA as sync_github_release_after_publish
participant PRS as print_release_summary
Rake->>PR: gem_version, dry_run, allow_version_policy_override
PR->>WRC: gem_root, dry_run
WRC-->>PR: sets released_gem_version and released_npm_version
alt dry_run = false
PR->>SGA: gem_root, sync_gem_version
SGA-->>PR: prints banner and sets changelog_section_found
end
PR-->>Rake: dry_run, released_gem_version, released_npm_version, changelog_section_found
Rake->>PRS: release_result hash
alt dry_run = true
PRS-->>Rake: DRY RUN COMPLETE plus version and file list
else changelog_section_found = true
PRS-->>Rake: RELEASE COMPLETE plus published versions and changelog check
else changelog_section_found = false
PRS-->>Rake: RELEASE COMPLETE plus published versions and next steps
end
Reviews (1): Last reviewed commit: "Add end-of-run summary for rake release" | Re-trigger Greptile |
|
Acknowledging skipped bot summary comments (#issuecomment-4176007942, #issuecomment-4176024710): these are informational status summaries, so no additional code or docs changes are required. |
ReviewNice improvement overall — the summary output makes the release process more ergonomic and the staged-files refactor is a clean win (using Shellwords.join instead of the hardcoded string is strictly better). Scope is appropriately narrow. Three things worth addressing before merging: 1. Duplicate next-steps guidance (medium) 2. Redundant extract_changelog_section call (low) 3. Dry-run summary omits changelog status (low) Checklist note |
- Pass pre-extracted changelog_section to sync_github_release_after_publish to avoid reading CHANGELOG.md twice with the same arguments. - Move changelog availability check outside `unless dry_run` so it runs for both paths, giving the dry-run summary a self-contained pre-flight check. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
|
Addressing the review summary (#issuecomment-4181568968):
|
| puts "Version would be bumped to: #{released_gem_version} (gem) / #{released_npm_version} (npm)" | ||
| puts "\nFiles that would be updated:" | ||
| staged_files.each { |file| puts " - #{file}" } | ||
| puts " - package.json (updated by release-it)" |
There was a problem hiding this comment.
The dry-run staged-files list is split between the dynamic release_staged_files array (lines above) and this hardcoded package.json line. If release_staged_files ever changes, this line also needs updating. Consider adding a comment or constant to make the coupling explicit, e.g.:
# package.json is managed by release-it and not pre-staged
puts " - package.json (updated by release-it)"Or surface it from release_staged_files so there's a single source of truth.
| npm_version = Shakapacker::Utils::VersionSyntaxConverter.new.rubygem_to_npm(gem_version) | ||
| changelog_path = File.join(gem_root, "CHANGELOG.md") | ||
| section = extract_changelog_section(changelog_path: changelog_path, npm_version: npm_version) | ||
| section = changelog_section || extract_changelog_section( |
There was a problem hiding this comment.
The changelog_section passed here is used only for the "should we skip?" guard (line ~293 unless section). However, prepare_github_release_context called further down always calls extract_changelog_section internally, so the file is still parsed a second time in the live path. The optimization avoids one redundant parse but doesn't fully thread the pre-extracted section all the way through — worth noting if this function is ever extended.
| end | ||
| end | ||
|
|
||
| { |
There was a problem hiding this comment.
released_gem_version and released_npm_version are set inside the with_release_checkout block. If the block aborts (e.g. a failed git add) they remain nil here. In practice abort prevents reaching this return statement, so it isn't a live bug — but the callers (print_release_summary) silently interpolate nil into strings if they were ever nil. A guard like released_gem_version || "(unknown)" in the summary printer would make failures more obvious.
Review: Add final summary output to rake releaseOverall this is a clean, well-scoped improvement. The refactoring to return metadata from Missing testsThe PR checklist leaves "Add/update test to cover these changes" unchecked.
|
* origin/main: (22 commits) docs: add Dependabot configuration guide (#1094) Sync address-review prompt with upstream PR #16 (#1098) Supersede #910: entry shape test with lint unblock (#919) fix: align rspack v2 peer deps and installer defaults (#1091) docs: update README and guides for Shakapacker v10 (#1092) Release 10.0.0 Update CHANGELOG.md for v10.0.0 (#1089) Release 10.0.0-rc.1 Update CHANGELOG.md for v10.0.0-rc.1 (#1087) Supersede #961 by using pack-config-diff (#973) Add final summary output to rake release (#1041) Add bin/setup to install development deps (#1039) Release 10.0.0-rc.0 Use npx release-it to avoid mise shim failures (#1040) Fix Nokogiri build failure on Ruby 3.4.6 (#1038) Update CHANGELOG.md for v10.0.0-rc.0 (#1037) Update rspack dev deps to 2.0.0-rc.0 (#1036) Fix stale and broken documentation across Shakapacker guides (#1023) Allow webpack-cli v7 in peer dependencies (#1021) refactor: simplify resolving js peer versions when installing (#1034) ... # Conflicts: # package.json
Summary
Adds an end-of-run summary for
rake releasein both dry-run and real release flows, matching the style used inreact_on_rails. It also updates the release flow to return release metadata so the summary can be printed after all release steps complete.Pull Request checklist
Update documentationUpdate CHANGELOG fileOther Information
Validated locally with
ruby -c rakelib/release.rake,bundle exec rake -T release, andbundle exec rubocop rakelib/release.rake.Note
Medium Risk
Changes the release automation path (staging, changelog lookup, and GitHub release sync), which can affect published artifacts and post-publish steps if the new metadata/summary logic is incorrect.
Overview
Adds a final, human-readable summary to
rake releasefor both dry runs and real releases, including the resolved gem/npm versions, which files were staged/updated, and whether a matchingCHANGELOG.mdsection exists (with next-step guidance when missing).Refactors the release flow to return release metadata (versions, staged files, changelog presence) and uses it to avoid re-parsing the changelog during GitHub release sync by passing an optional pre-extracted
changelog_section. Staging of release-related files is centralized viarelease_staged_filesand staged using a generatedgit addcommand.Written by Cursor Bugbot for commit 269a4a9. This will update automatically on new commits. Configure here.
Summary by CodeRabbit