Modernize release process for monorepo #1856
Conversation
- Replace release-it with native Yarn workspace publish commands - Update release rake task to handle both NPM packages (react-on-rails and react-on-rails-pro) - Synchronize versions across gem and both NPM packages - Set react-on-rails-pro to use exact version dependency on react-on-rails - Remove obsolete release scripts from package.json files - Delete packages/react-on-rails/scripts/release - Update documentation with comprehensive release instructions The new release process: 1. Bumps version in lib/react_on_rails/version.rb (source of truth) 2. Updates all 3 package.json files with new version 3. Updates react-on-rails-pro dependency to exact version 4. Commits, tags, and pushes changes 5. Publishes both NPM packages using yarn workspace publish 6. Publishes Ruby gem Benefits: - No external release-it dependency - Both NPM packages properly released - Version sync enforced across all 5 files - Single command releases everything: rake release[X.Y.Z] 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
|
Warning Rate limit exceeded@AbanoubGhadban has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 19 minutes and 49 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughReplaces local npm release scripts with a unified Rake-driven release flow that reads/bumps the Ruby gem version, derives and propagates an npm semver to root and package package.json files, updates inter-package dependency, and orchestrates git/tag/push and publishing to npm (or Verdaccio) and optionally RubyGems. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant Rake as rakelib/release.rake
participant Git as Git
participant NPM as npm Registry (npmjs / Verdaccio)
participant Gem as RubyGems
participant Root as root/package.json
participant ROR as packages/react-on-rails/package.json
participant RORPro as packages/react-on-rails-pro/package.json
Dev->>Rake: rake release[gem_version, dry_run, registry, skip_push]
Rake->>Git: git pull --rebase
Rake->>Rake: bump or read gem version (lib/react_on_rails/version.rb)
Rake->>Rake: derive npm semver from gem version
Rake->>Root: update root/package.json version
Rake->>ROR: update packages/react-on-rails/package.json version
Rake->>RORPro: update packages/react-on-rails-pro/package.json version & pin dependency
alt dry-run
Rake-->>Dev: show diffs and commands (no pushes/publishes)
else execute
Rake->>Git: git add/commit changes
Rake->>Git: git tag
alt skip_push == false
Rake->>Git: git push && git push --tags
end
Rake->>NPM: publish npm packages (OTP prompt if required)
alt registry == Verdaccio
Note right of NPM: Verdaccio mode — skip gem publish
else
Rake->>Gem: push gem to RubyGems
end
Rake-->>Dev: summary and post-release instructions
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 |
Code Review: Modernize Release Process for Monorepo✅ Strengths
🐛 Potential Issues1. Critical: Error Handling in Version Extraction (rakelib/release.rake:57-61)The regex extraction could fail if the version format changes or the file is malformed. If match returns nil, accessing [1] will raise NoMethodError and the release will fail in a confusing way. Recommendation: Add error handling to raise a clear error message if version extraction fails. 2. Potential Race Condition: Git Push Before Publish (rakelib/release.rake:100-101)The code pushes commits and tags to the remote repository BEFORE actually publishing to NPM and RubyGems. If NPM or RubyGems publish fails (network issues, 2FA timeout, permissions), the git tag and version bump are already public, but packages aren't published. This creates an inconsistent state. Recommendation: Consider pushing after successful publish, or at minimum add a rollback mechanism. 3. JSON Formatting Inconsistency (rakelib/release.rake:84)The code uses JSON.pretty_generate which may not match the existing formatting style in package.json files (indentation, spacing, key ordering). This could create large diffs with formatting changes unrelated to the version bump. Since CLAUDE.md emphasizes Prettier as the sole authority for formatting, this could cause conflicts. Recommendation: Run Prettier after modifying package.json files to ensure consistent formatting. 4. Missing Validation: Yarn Publish Command (rakelib/release.rake:110, 115)The yarn workspace publish commands don't verify success before proceeding. If the first publish fails or is cancelled, the script continues and may partially succeed, leaving an inconsistent state.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
rakelib/release.rake (1)
41-49: Dry-run currently mutates the repo (gem bump, JSON writes, bundle, git pull)This breaks the promise of a “preview only” mode and risks a dirty working tree. Gate all mutating steps behind
unless is_dry_run, and compute the target versions without writing files in dry runs.Apply these diffs:
- Gate the pre-steps and
git pull:- # Having the examples prevents publishing - Rake::Task["shakapacker_examples:clobber"].invoke - # Delete any react_on_rails.gemspec except the root one - sh_in_dir(gem_root, "find . -mindepth 2 -name 'react_on_rails.gemspec' -delete") - - # Pull latest changes - sh_in_dir(gem_root, "git pull --rebase") + unless is_dry_run + # Having the examples prevents publishing + Rake::Task["shakapacker_examples:clobber"].invoke + # Delete any react_on_rails.gemspec except the root one + sh_in_dir(gem_root, "find . -mindepth 2 -name 'react_on_rails.gemspec' -delete") + # Pull latest changes + sh_in_dir(gem_root, "git pull --rebase") + end
- Compute
actual_gem_versionwithout mutating in dry-run; only bump when not dry-run:- # Bump gem version using gem-release - sh_in_dir(gem_root, "gem bump --no-commit #{%(--version #{gem_version}) unless gem_version.strip.empty?}") - - # Read the actual version that was set - actual_gem_version = begin - version_file = File.join(gem_root, "lib", "react_on_rails", "version.rb") - version_content = File.read(version_file) - version_content.match(/VERSION = "(.+)"/)[1] - end + version_file = File.join(gem_root, "lib", "react_on_rails", "version.rb") + if is_dry_run + current = File.read(version_file) + m = current.match(/VERSION\s*=\s*"(.+?)"/) + raise "Could not parse version.rb for VERSION" unless m + current_gem_version = m[1] + actual_gem_version = + if gem_version.to_s.strip.empty? + parts = current_gem_version.split(".") + parts[2] = (parts[2].to_i + 1).to_s + parts.join(".") + else + gem_version.strip + end + else + # Bump gem version using gem-release + sh_in_dir(gem_root, "gem bump --no-commit #{%(--version #{gem_version}) unless gem_version.to_s.strip.empty?}") + # Read the actual version that was set + version_content = File.read(version_file) + m = version_content.match(/VERSION\s*=\s*"(.+?)"/) + raise "Could not parse version.rb for VERSION" unless m + actual_gem_version = m[1] + end
- Gate package.json writes and Bundler lockfile updates:
- puts "Updating package.json files to version #{actual_npm_version}..." - - # Update all package.json files - package_json_files = [ - File.join(gem_root, "package.json"), - File.join(gem_root, "packages", "react-on-rails", "package.json"), - File.join(gem_root, "packages", "react-on-rails-pro", "package.json") - ] - - package_json_files.each do |file| - content = JSON.parse(File.read(file)) - content["version"] = actual_npm_version - - # For react-on-rails-pro, also update the react-on-rails dependency - if file.include?("react-on-rails-pro") - content["dependencies"] ||= {} - content["dependencies"]["react-on-rails"] = actual_npm_version - end - - File.write(file, "#{JSON.pretty_generate(content)}\n") - puts " Updated #{file}" - end - - # Update dummy app's Gemfile.lock - bundle_install_in(dummy_app_dir) + unless is_dry_run + puts "Updating package.json files to version #{actual_npm_version}..." + package_json_files = [ + File.join(gem_root, "package.json"), + File.join(gem_root, "packages", "react-on-rails", "package.json"), + File.join(gem_root, "packages", "react-on-rails-pro", "package.json") + ] + package_json_files.each do |file| + content = JSON.parse(File.read(file)) + content["version"] = actual_npm_version + if file.include?("react-on-rails-pro") + content["dependencies"] ||= {} + content["dependencies"]["react-on-rails"] = actual_npm_version + end + File.write(file, "#{JSON.pretty_generate(content)}\n") + puts " Updated #{file}" + end + # Update dummy app's Gemfile.lock + bundle_install_in(dummy_app_dir) + endAlso applies to: 50-55, 56-61, 65-90
🧹 Nitpick comments (3)
docs/contributor-info/releasing.md (2)
147-151: Avoidyarn publish --new-versionin manual recovery stepsUsing
--new-versioncan re-tag/re-commit and conflict with the earlier tag/commit. Recommend publishing the already-versioned workspaces withnpm publishinstead:
cd packages/react-on-rails && npm publish --access publiccd ../react-on-rails-pro && npm publish --access public
157-185: Add language to fenced code block (markdownlint MD040)Annotate the code fence as diff.
-``` +```diff commit abc123... ... } -``` +```rakelib/release.rake (1)
78-82: Confirm dependency field for Pro → OSS couplingYou’re forcing an exact version under
dependencies. If the intention is to require the app to install the same OSS version alongside Pro, this is OK. If Pro should be compatible with a range in caller apps, considerpeerDependencies(withpeerDependenciesMeta.optionalas needed) to avoid duplicate installs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/contributor-info/releasing.md(1 hunks)package.json(0 hunks)packages/react-on-rails/package.json(1 hunks)packages/react-on-rails/scripts/release(0 hunks)rakelib/release.rake(3 hunks)
💤 Files with no reviewable changes (2)
- packages/react-on-rails/scripts/release
- package.json
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,jsx,ts,tsx,css,scss,json,yml,yaml,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Prettier is the sole authority for formatting all non-Ruby files; never manually format them
Files:
packages/react-on-rails/package.jsondocs/contributor-info/releasing.md
{Gemfile,Rakefile,config.ru,**/*.{rb,rake,gemspec,ru}}
📄 CodeRabbit inference engine (CLAUDE.md)
{Gemfile,Rakefile,config.ru,**/*.{rb,rake,gemspec,ru}}: All Ruby code must pass RuboCop with zero offenses before commit/push
RuboCop is the sole authority for Ruby file formatting; never manually format Ruby files
Files:
rakelib/release.rake
🪛 markdownlint-cli2 (0.18.1)
docs/contributor-info/releasing.md
157-157: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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). (10)
- GitHub Check: dummy-app-integration-tests (3.2, 20)
- GitHub Check: dummy-app-integration-tests (3.4, 22)
- GitHub Check: rspec-package-tests (3.4, latest)
- GitHub Check: rspec-package-tests (3.4, minimum)
- GitHub Check: rspec-package-tests (3.2, minimum)
- GitHub Check: rspec-package-tests (3.2, latest)
- GitHub Check: markdown-link-check
- GitHub Check: claude-review
- GitHub Check: build
- GitHub Check: build-and-test
🔇 Additional comments (1)
packages/react-on-rails/package.json (1)
17-18: Removal of release scripts looks goodChange aligns with the unified rake-based release flow.
- Remove extra end statement that was causing syntax error - Fix string concatenation to use interpolation - Fix operator precedence ambiguity - Convert multi-line unless blocks to modifier form - Refactor unless/else to if/else for clarity - Fix line length violation - Eliminate duplicate separator line code All RuboCop violations resolved. File now passes bundle exec rubocop with zero offenses.
- Add 4th argument 'skip_push' to skip git push operations - Useful for testing the release process locally - Commits and tags are still created, but not pushed to remote - Displays warning message when push is skipped - Update documentation with new parameter and example usage Example: rake release[16.2.0,false,npm,skip_push] 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Code Review: Modernize Release Process for MonorepoOverall, this is a well-architected improvement that modernizes the release process. The changes are logical, well-documented, and reduce external dependencies. Strengths1. Excellent Simplification
2. Strong Documentation
3. Version Synchronization
4. Verdaccio Support
Issues & RecommendationsCritical Issues1. Regex Vulnerability (rakelib/release.rake:64) 2. Lack of Error Handling (rakelib/release.rake:111-149) 3. Missing JSON Parse Validation (rakelib/release.rake:79) Medium Priority4. Missing step 2 in documentation (line 201) 5. Race condition with git pull (line 55) 6. No verification of successful publish (lines 130-136) Minor Issues
SecurityNo major security issues detected:
Test CoverageConcern: This PR does not include tests for the new release task. Recommendations:
PerformancePerformance looks good:
Additional Suggestions
CLAUDE.md Compliance
Note: Make sure to run bundle exec rubocop before committing (as per CLAUDE.md requirements). SummaryThis is a high-quality PR that meaningfully improves the release process. The main concerns are:
With these issues addressed, this will be an excellent improvement to the project. Great work! |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
rakelib/release.rake (2)
60-65: Harden version parsing from version.rbIf the regex doesn't match,
match(...)[1]will raise. Add a nil-check and clearer error.Apply this diff:
- actual_gem_version = begin - version_file = File.join(gem_root, "lib", "react_on_rails", "version.rb") - version_content = File.read(version_file) - version_content.match(/VERSION = "(.+)"/)[1] - end + actual_gem_version = begin + version_file = File.join(gem_root, "lib", "react_on_rails", "version.rb") + version_content = File.read(version_file) + m = version_content.match(/VERSION\s*=\s*"([^"]+)"/) + raise "Could not parse VERSION from #{version_file}" unless m + m[1] + end
130-136: Prevent duplicate tagging when publishing.
yarn workspace ... publish --new-versionwill bump package.json, commit, and tag again after you've already done so. Publish the already-versioned packages directly.Apply this diff:
- sh_in_dir(gem_root, "yarn workspace react-on-rails publish --new-version #{actual_npm_version} #{npm_publish_args}") + sh_in_dir(File.join(gem_root, "packages", "react-on-rails"), "npm publish --access public #{npm_publish_args}") - sh_in_dir(gem_root, - "yarn workspace react-on-rails-pro publish --new-version #{actual_npm_version} #{npm_publish_args}") + sh_in_dir(File.join(gem_root, "packages", "react-on-rails-pro"), "npm publish --access public #{npm_publish_args}")Alternatively, if you must use Yarn workspace commands, add
--no-git-tag-version:- sh_in_dir(gem_root, "yarn workspace react-on-rails publish --new-version #{actual_npm_version} #{npm_publish_args}") + sh_in_dir(gem_root, "yarn workspace react-on-rails publish --new-version #{actual_npm_version} --no-git-tag-version #{npm_publish_args}") - sh_in_dir(gem_root, - "yarn workspace react-on-rails-pro publish --new-version #{actual_npm_version} #{npm_publish_args}") + sh_in_dir(gem_root, + "yarn workspace react-on-rails-pro publish --new-version #{actual_npm_version} --no-git-tag-version #{npm_publish_args}")
🧹 Nitpick comments (1)
rakelib/release.rake (1)
148-148: Verify gem-release availability before use.The task relies on the gem-release gem but doesn't verify it's available. Consider adding an early check to provide a clearer error message.
Add this check at the beginning of the task (after line 46):
begin require "gem/release" rescue LoadError abort "Error: gem-release is required. Install it with: gem install gem-release" end
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
rakelib/release.rake(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
{Gemfile,Rakefile,config.ru,**/*.{rb,rake,gemspec,ru}}
📄 CodeRabbit inference engine (CLAUDE.md)
{Gemfile,Rakefile,config.ru,**/*.{rb,rake,gemspec,ru}}: All Ruby code must pass RuboCop with zero offenses before commit/push
RuboCop is the sole authority for Ruby file formatting; never manually format Ruby files
Files:
rakelib/release.rake
⏰ 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). (10)
- GitHub Check: dummy-app-integration-tests (3.4, 22)
- GitHub Check: dummy-app-integration-tests (3.2, 20)
- GitHub Check: claude-review
- GitHub Check: build
- GitHub Check: markdown-link-check
- GitHub Check: rspec-package-tests (3.2, latest)
- GitHub Check: rspec-package-tests (3.4, latest)
- GitHub Check: rspec-package-tests (3.4, minimum)
- GitHub Check: rspec-package-tests (3.2, minimum)
- GitHub Check: build-and-test
🔇 Additional comments (4)
rakelib/release.rake (4)
18-36: Well-documented task interface.The updated documentation clearly explains the release process for both NPM packages and the gem, including version format conversion, dry run support, and Verdaccio testing.
78-90: Excellent version synchronization logic.The implementation correctly updates all package.json files and properly synchronizes the react-on-rails dependency version in react-on-rails-pro to maintain exact version matching.
95-109: Verdaccio support is well-implemented.The conditional registry configuration with clear instructions and user confirmation provides a good developer experience for testing releases locally.
158-208: Excellent dry run and success messaging.The comprehensive output clearly shows what will be modified and provides helpful next steps, with appropriate differentiation between Verdaccio test mode and production releases.
rakelib/release.rake
Outdated
| sh_in_dir(gem_root, "git add -A") | ||
| sh_in_dir(gem_root, "git commit -m 'Bump version to #{actual_gem_version}'") | ||
|
|
||
| # Create git tag | ||
| sh_in_dir(gem_root, "git tag v#{actual_gem_version}") | ||
|
|
||
| # Push commits and tags | ||
| sh_in_dir(gem_root, "git push") | ||
| sh_in_dir(gem_root, "git push --tags") |
There was a problem hiding this comment.
Add error handling for git operations.
If git push or git push --tags fails after local commits/tags are created, the repository is left in an inconsistent state with no rollback mechanism. Consider wrapping these operations in error handling that provides guidance on manual recovery.
Example implementation:
begin
sh_in_dir(gem_root, "git push")
sh_in_dir(gem_root, "git push --tags")
rescue StandardError => e
puts "\n❌ Git push failed: #{e.message}"
puts "Local commits and tags created but not pushed."
puts "To recover:"
puts " 1. Fix the issue (e.g., network, permissions)"
puts " 2. Run: git push && git push --tags"
puts " 3. Then manually publish packages"
abort
end🤖 Prompt for AI Agents
In rakelib/release.rake around lines 113 to 121, the git push and git push
--tags commands are executed without error handling which can leave local
commits and tags unpushed; wrap those sh_in_dir calls in a begin/rescue that
catches StandardError, prints a clear error message including e.message and
recovery steps (explain the local state, advise fixing network/permission
issues, and provide the exact git push && git push --tags command to run), and
then abort the rake task to prevent further release steps from proceeding.
| if use_verdaccio | ||
| puts "Skipping Ruby gem publication (Verdaccio mode)" | ||
| puts "=" * 80 | ||
| else | ||
| sh_in_dir(gem_root, "git commit -m 'Update Gemfile.lock for version #{gem_version}'") | ||
| puts "\n#{'=' * 80}" | ||
| puts "Publishing Ruby gem..." | ||
| puts "=" * 80 | ||
|
|
||
| # Publish Ruby gem | ||
| puts "\nCarefully add your OTP for Rubygems when prompted." | ||
| sh_in_dir(gem_root, "gem release") | ||
| end |
There was a problem hiding this comment.
Add error handling for gem publication.
If gem release fails after NPM packages are already published, the release is left in an inconsistent state. Consider adding error handling to guide manual recovery.
Example implementation:
begin
puts "\nCarefully add your OTP for Rubygems when prompted."
sh_in_dir(gem_root, "gem release")
rescue StandardError => e
puts "\n❌ Gem release failed: #{e.message}"
puts "NPM packages already published:"
puts " - react-on-rails@#{actual_npm_version}"
puts " - react-on-rails-pro@#{actual_npm_version}"
puts "To complete the release:"
puts " 1. Fix the issue"
puts " 2. Run: gem release"
abort
end🤖 Prompt for AI Agents
In rakelib/release.rake around lines 138 to 149, the gem publication step runs
`gem release` without error handling which can leave the release in an
inconsistent state if it fails after NPM packages were published; wrap the
`sh_in_dir(gem_root, "gem release")` call in a begin/rescue that catches
StandardError, prints a clear failure message including the exception message,
lists the already-published NPM packages and their versions (use the existing
variable holding the actual_npm_version), instructs the user how to recover (fix
issue then run `gem release`), and aborts the rake task to stop further
processing.
- Validate registry parameter accepts only: 'verdaccio', 'npm', or empty string - Validate skip_push parameter accepts only: 'skip_push' or empty string - Raise ArgumentError with clear message for invalid values - Prevents silent failures from typos or incorrect usage Examples of errors: rake release[16.2.0,false,foo] → ArgumentError: Invalid registry value 'foo' rake release[16.2.0,false,npm,skip] → ArgumentError: Invalid skip_push value 'skip' 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
This is a well-structured modernization of the release process. Key strengths: excellent documentation, proper version synchronization across all packages, developer-friendly features (dry run, Verdaccio, skip push), and simplified architecture by removing release-it dependency. Main concerns: (1) CRITICAL - Missing rollback mechanism if NPM publishing fails after git push, creating version inconsistency. Recommend pushing git changes AFTER successful package publishing or adding recovery task. (2) git add -A is too broad and may stage unintended WIP files. (3) Missing automated tests for critical release logic. (4) Regex pattern for version extraction could be more robust. Overall a solid improvement that would be ready to merge after addressing the rollback issue. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
rakelib/release.rake (4)
64-69: Harden version parsing from version.rbIf the regex doesn't match,
match(...)[1]will raiseNoMethodError. Add a nil-check and a clearer error message.Apply this diff to add error handling:
- actual_gem_version = begin - version_file = File.join(gem_root, "lib", "react_on_rails", "version.rb") - version_content = File.read(version_file) - version_content.match(/VERSION = "(.+)"/)[1] - end + actual_gem_version = begin + version_file = File.join(gem_root, "lib", "react_on_rails", "version.rb") + version_content = File.read(version_file) + m = version_content.match(/VERSION\s*=\s*"(.+?)"/) + raise "Could not parse VERSION from #{version_file}. Check version.rb formatting." unless m + m[1] + end
136-136: Prevent duplicate tagging when publishing NPM packages
yarn workspace ... publish --new-versionwill bump package.json, create a commit, and tag again. Since you've already updated versions (lines 82-94), committed (line 118), and tagged (line 121), this will cause duplicate tags or failures.Apply this diff to publish the already-versioned packages:
- sh_in_dir(gem_root, "yarn workspace react-on-rails publish --new-version #{actual_npm_version} #{npm_publish_args}") + sh_in_dir(gem_root, "yarn workspace react-on-rails publish #{npm_publish_args}")- sh_in_dir(gem_root, - "yarn workspace react-on-rails-pro publish --new-version #{actual_npm_version} #{npm_publish_args}") + sh_in_dir(gem_root, "yarn workspace react-on-rails-pro publish #{npm_publish_args}")If you must keep
--new-version, add--no-git-tag-versionto prevent re-tagging.Also applies to: 141-142
125-127: Add error handling for git operationsIf
git pushorgit push --tagsfails after local commits/tags are created, the repository is left in an inconsistent state with no rollback mechanism.Apply this diff to add recovery guidance:
- unless skip_push - sh_in_dir(gem_root, "git push") - sh_in_dir(gem_root, "git push --tags") - end + unless skip_push + begin + sh_in_dir(gem_root, "git push") + sh_in_dir(gem_root, "git push --tags") + rescue StandardError => e + puts "\n❌ Git push failed: #{e.message}" + puts "Local commits and tags created but not pushed." + puts "To recover:" + puts " 1. Fix the issue (e.g., network, permissions)" + puts " 2. Run: git push && git push --tags" + puts " 3. Then manually complete release: rake release[#{actual_gem_version}]" + abort + end + end
153-154: Add error handling for gem publicationIf
gem releasefails after NPM packages are already published, the release is left in an inconsistent state.Apply this diff to add recovery guidance:
- puts "\nCarefully add your OTP for Rubygems when prompted." - sh_in_dir(gem_root, "gem release") + begin + puts "\nCarefully add your OTP for Rubygems when prompted." + sh_in_dir(gem_root, "gem release") + rescue StandardError => e + puts "\n❌ Gem release failed: #{e.message}" + puts "NPM packages already published:" + puts " - react-on-rails@#{actual_npm_version}" + puts " - react-on-rails-pro@#{actual_npm_version}" + puts "To complete the release:" + puts " 1. Fix the issue" + puts " 2. Run: gem release" + abort + end
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
rakelib/release.rake(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
{Gemfile,Rakefile,config.ru,**/*.{rb,rake,gemspec,ru}}
📄 CodeRabbit inference engine (CLAUDE.md)
{Gemfile,Rakefile,config.ru,**/*.{rb,rake,gemspec,ru}}: All Ruby code must pass RuboCop with zero offenses before commit/push
RuboCop is the sole authority for Ruby file formatting; never manually format Ruby files
Files:
rakelib/release.rake
⏰ 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). (10)
- GitHub Check: dummy-app-integration-tests (3.2, 20)
- GitHub Check: dummy-app-integration-tests (3.4, 22)
- GitHub Check: rspec-package-tests (3.4, minimum)
- GitHub Check: rspec-package-tests (3.2, latest)
- GitHub Check: rspec-package-tests (3.4, latest)
- GitHub Check: rspec-package-tests (3.2, minimum)
- GitHub Check: build
- GitHub Check: markdown-link-check
- GitHub Check: claude-review
- GitHub Check: build-and-test
| Next steps: | ||
| 1. Update CHANGELOG.md: bundle exec rake update_changelog | ||
| 3. Commit CHANGELOG: cd #{gem_root} && git commit -a -m 'Update CHANGELOG.md and spec/dummy Gemfile.lock' |
There was a problem hiding this comment.
Fix step numbering
The step numbering skips from 1 to 3 (missing step 2).
Apply this diff:
Next steps:
1. Update CHANGELOG.md: bundle exec rake update_changelog
- 3. Commit CHANGELOG: cd #{gem_root} && git commit -a -m 'Update CHANGELOG.md and spec/dummy Gemfile.lock'
- 4. Push changes: git push
+ 2. Commit CHANGELOG: cd #{gem_root} && git commit -a -m 'Update CHANGELOG.md and spec/dummy Gemfile.lock'
+ 3. Push changes: git push📝 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.
| 3. Commit CHANGELOG: cd #{gem_root} && git commit -a -m 'Update CHANGELOG.md and spec/dummy Gemfile.lock' | |
| Next steps: | |
| 1. Update CHANGELOG.md: bundle exec rake update_changelog | |
| 2. Commit CHANGELOG: cd #{gem_root} && git commit -a -m 'Update CHANGELOG.md and spec/dummy Gemfile.lock' | |
| 3. Push changes: git push |
🤖 Prompt for AI Agents
In rakelib/release.rake around line 217 the numbered step jumps from "1." to
"3." so update the sequence to use the correct numbering (change "3. Commit
CHANGELOG: cd #{gem_root} && git commit -a -m 'Update CHANGELOG.md and
spec/dummy Gemfile.lock'" to use "2." instead) so the steps read 1, 2, 3 in
order; ensure any adjacent step numbers are consistent if present.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (5)
rakelib/release.rake (5)
79-84: Harden version parsing from version.rb.If the regex doesn't match,
match(...)[1]will raise. Add a nil-check and clearer error.Apply this diff:
- actual_gem_version = begin - version_file = File.join(gem_root, "lib", "react_on_rails", "version.rb") - version_content = File.read(version_file) - version_content.match(/VERSION = "(.+)"/)[1] - end + actual_gem_version = begin + version_file = File.join(gem_root, "lib", "react_on_rails", "version.rb") + version_content = File.read(version_file) + m = version_content.match(/VERSION\s*=\s*"(.+?)"/) + raise "Could not parse version.rb for VERSION" unless m + m[1] + end
138-142: Add error handling for git operations.If
git pushorgit push --tagsfails after local commits/tags are created, the repository is left in an inconsistent state with no rollback mechanism.Example implementation:
begin unless skip_push sh_in_dir(gem_root, "git push") sh_in_dir(gem_root, "git push --tags") end rescue StandardError => e puts "\n❌ Git push failed: #{e.message}" puts "Local commits and tags created but not pushed." puts "To recover:" puts " 1. Fix the issue (e.g., network, permissions)" puts " 2. Run: git push && git push --tags" puts " 3. Then manually publish packages" abort end
148-157: Prevent duplicate tagging when publishing.Yarn's
publish --new-versionwill bump package.json, commit, and tag again. The package.json files are already updated (lines 97-109) and committed (line 133), so this causes duplicate commits/tags.Publish the already-versioned packages directly:
- sh_in_dir(gem_root, "yarn workspace react-on-rails publish --new-version #{actual_npm_version} #{npm_publish_args}") + sh_in_dir(File.join(gem_root, "packages", "react-on-rails"), "npm publish --access public #{npm_publish_args}")- sh_in_dir(gem_root, - "yarn workspace react-on-rails-pro publish --new-version #{actual_npm_version} #{npm_publish_args}") + sh_in_dir(File.join(gem_root, "packages", "react-on-rails-pro"), "npm publish --access public #{npm_publish_args}")If you must use Yarn, add
--no-git-tag-versionto prevent re-bumping/tagging.
167-169: Add error handling for gem publication.If
gem releasefails after NPM packages are already published, the release is left in an inconsistent state.Example implementation:
begin puts "\nCarefully add your OTP for Rubygems when prompted." sh_in_dir(gem_root, "gem release") rescue StandardError => e puts "\n❌ Gem release failed: #{e.message}" puts "NPM packages already published:" puts " - react-on-rails@#{actual_npm_version}" puts " - react-on-rails-pro@#{actual_npm_version}" puts "To complete the release:" puts " 1. Fix the issue" puts " 2. Run: gem release" abort end
232-232: Fix step numbering.The step numbering skips from 1 to 3 (missing step 2).
Apply this diff:
Next steps: 1. Update CHANGELOG.md: bundle exec rake update_changelog - 3. Commit CHANGELOG: cd #{gem_root} && git commit -a -m 'Update CHANGELOG.md and spec/dummy Gemfile.lock' - 4. Push changes: git push + 2. Commit CHANGELOG: cd #{gem_root} && git commit -a -m 'Update CHANGELOG.md and spec/dummy Gemfile.lock' + 3. Push changes: git push
🧹 Nitpick comments (1)
rakelib/release.rake (1)
118-128: Consider non-interactive environment handling.The
$stdin.getsat line 127 will block in non-interactive environments (e.g., CI/CD pipelines). Consider adding a check for TTY availability or an environment variable to bypass the prompt:if use_verdaccio puts "\n#{'=' * 80}" puts "VERDACCIO LOCAL REGISTRY MODE" puts "=" * 80 puts "\nBefore proceeding, ensure:" puts " 1. Verdaccio server is running on http://localhost:4873/" puts " 2. You are authenticated with Verdaccio:" puts " npm adduser --registry http://localhost:4873/" puts "\nPress ENTER to continue or Ctrl+C to cancel..." $stdin.gets if $stdin.tty? && !is_dry_run end
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
rakelib/release.rake(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
{Gemfile,Rakefile,config.ru,**/*.{rb,rake,gemspec,ru}}
📄 CodeRabbit inference engine (CLAUDE.md)
{Gemfile,Rakefile,config.ru,**/*.{rb,rake,gemspec,ru}}: All Ruby code must pass RuboCop with zero offenses before commit/push
RuboCop is the sole authority for Ruby file formatting; never manually format Ruby files
Files:
rakelib/release.rake
⏰ 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: build-and-test
🔇 Additional comments (5)
rakelib/release.rake (5)
1-9: LGTM!The addition of
require "json"is necessary for the package.json manipulation logic introduced later in the file.
18-40: LGTM!The expanded task signature with comprehensive documentation and examples helps users understand the new Verdaccio and skip_push features.
49-64: LGTM!Parameter validation for
registryandskip_pushis implemented correctly with clear error messages for invalid values.
73-74: Verify git pull timing relative to uncommitted changes check.The uncommitted changes check occurs at line 44, but
git pull --rebaseis executed here. If the pull introduces conflicts or new changes, those won't be validated by the earlier check. Consider moving the git pull before the uncommitted changes check, or re-checking after the pull.
86-109: LGTM!The package.json update logic correctly:
- Updates version in all three package.json files
- Updates the react-on-rails dependency version in react-on-rails-pro
- Preserves formatting with pretty_generate and trailing newline
Code Review: Modernize Release Process for MonorepoOverall Assessment ✅This is a well-executed modernization of the release process. The PR successfully consolidates the release workflow, eliminates external dependencies, and provides excellent documentation. 🎯 Strengths
🐛 Critical Issues1. Version Mismatch in Pro Package
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
rakelib/release.rake (5)
79-84: Harden version parsing from version.rb.The regex match can raise if the VERSION constant format is unexpected. This issue was previously flagged but remains unaddressed.
Add nil-check as previously suggested:
- version_content = File.read(version_file) - version_content.match(/VERSION = "(.+)"/)[1] + version_content = File.read(version_file) + m = version_content.match(/VERSION\s*=\s*"(.+?)"/) + raise "Could not parse VERSION from #{version_file}" unless m + m[1]
152-158: Prevent duplicate git operations when publishing.Using
yarn workspace ... publish --new-versionwill bump package.json, commit, and tag again after you've already done so at lines 133-137. This issue was previously flagged but remains unaddressed.Apply the previously suggested fix to publish without re-versioning:
- sh_in_dir(gem_root, "yarn workspace react-on-rails publish --new-version #{actual_npm_version} #{npm_publish_args}") + sh_in_dir(File.join(gem_root, "packages", "react-on-rails"), "npm publish --access public #{npm_publish_args}") - sh_in_dir(gem_root, - "yarn workspace react-on-rails-pro publish --new-version #{actual_npm_version} #{npm_publish_args}") + sh_in_dir(File.join(gem_root, "packages", "react-on-rails-pro"), "npm publish --access public #{npm_publish_args}")Alternatively, use
--no-git-tag-versionif you prefer staying with yarn:- sh_in_dir(gem_root, "yarn workspace react-on-rails publish --new-version #{actual_npm_version} #{npm_publish_args}") + sh_in_dir(gem_root, "yarn workspace react-on-rails publish --new-version #{actual_npm_version} --no-git-tag-version #{npm_publish_args}")
141-142: Add error handling for git push operations.Git push failures leave local commits/tags unpushed with no recovery guidance. This issue was previously flagged but remains unaddressed.
Wrap in error handling as previously suggested:
begin sh_in_dir(gem_root, "git push") sh_in_dir(gem_root, "git push --tags") rescue StandardError => e puts "\n❌ Git push failed: #{e.message}" puts "Local commits and tags created but not pushed." puts "To recover:" puts " 1. Fix the issue (e.g., network, permissions)" puts " 2. Run: git push && git push --tags" puts " 3. Then manually publish packages" abort end
170-170: Add error handling for gem publication.Gem release failures leave npm packages published with no gem. This issue was previously flagged but remains unaddressed.
Wrap in error handling as previously suggested:
begin puts "\nCarefully add your OTP for Rubygems when prompted." sh_in_dir(gem_root, "gem release") rescue StandardError => e puts "\n❌ Gem release failed: #{e.message}" puts "NPM packages already published:" puts " - react-on-rails@#{actual_npm_version}" puts " - react-on-rails-pro@#{actual_npm_version}" puts "To complete the release:" puts " 1. Fix the issue" puts " 2. Run: gem release" abort end
233-233: Fix step numbering.Steps jump from 1 to 3. This issue was previously flagged but remains unaddressed.
- 3. Commit CHANGELOG: cd #{gem_root} && git commit -a -m 'Update CHANGELOG.md and spec/dummy Gemfile.lock' - 4. Push changes: git push + 2. Commit CHANGELOG: cd #{gem_root} && git commit -a -m 'Update CHANGELOG.md and spec/dummy Gemfile.lock' + 3. Push changes: git push
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/react-on-rails-pro/package.json(1 hunks)rakelib/release.rake(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
{Gemfile,Rakefile,config.ru,**/*.{rb,rake,gemspec,ru}}
📄 CodeRabbit inference engine (CLAUDE.md)
{Gemfile,Rakefile,config.ru,**/*.{rb,rake,gemspec,ru}}: All Ruby code must pass RuboCop with zero offenses before commit/push
RuboCop is the sole authority for Ruby file formatting; never manually format Ruby files
Files:
rakelib/release.rake
**/*.{js,jsx,ts,tsx,css,scss,json,yml,yaml,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Prettier is the sole authority for formatting all non-Ruby files; never manually format them
Files:
packages/react-on-rails-pro/package.json
⏰ 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). (7)
- GitHub Check: claude-review
- GitHub Check: rspec-package-tests (3.4, minimum)
- GitHub Check: rspec-package-tests (3.4, latest)
- GitHub Check: rspec-package-tests (3.2, latest)
- GitHub Check: rspec-package-tests (3.2, minimum)
- GitHub Check: markdown-link-check
- GitHub Check: build-and-test
🔇 Additional comments (1)
rakelib/release.rake (1)
1-242: Overall modernization approach looks solid.The refactored release process successfully:
- Centralizes version management in Ruby gem version as source of truth
- Synchronizes versions across gem and both npm packages
- Adds Verdaccio support for local testing
- Provides skip_push for testing without remote operations
- Includes comprehensive dry-run and success messaging
However, several issues from previous reviews remain unaddressed and should be resolved before merging.
Code Review SummaryThis PR successfully modernizes the release process to properly handle the monorepo structure. The changes are well-structured and significantly improve the release workflow. Here's my detailed feedback: ✅ Strengths1. Excellent Code Quality
2. Strong Documentation
3. Great Features
4. Proper Monorepo Handling
🔍 Potential Issues & SuggestionsCritical: Potential Regex Failure
|
Summary
Modernizes the release process to properly handle the monorepo structure with both NPM packages (
react-on-railsandreact-on-rails-pro) and the Ruby gem.Changes
🔧 Updated Release Workflow
release-itwith native Yarn workspace publish commandsrakelib/release.raketo handle both NPM packages plus the gemlib/react_on_rails/version.rb(source of truth)package.json(root)packages/react-on-rails/package.jsonpackages/react-on-rails-pro/package.json(version + dependency)spec/dummy/Gemfile.lock📦 Package Dependencies
react-on-rails-pronow depends on the exact version ofreact-on-rails(e.g.,"16.1.1"not"*")🗑️ Cleanup
packages/react-on-rails/scripts/releasescriptrelease:patch,release:minor,release:majorscripts from package.json files📚 Documentation
docs/contributor-info/releasing.mdwith:Release Task Options
The
rake releasetask now supports comprehensive options for different release scenarios:Parameters
gem_version(optional): Version in rubygem format (e.g.,16.2.0). Omit for auto patch bump.dry_run(optional): Passtrueto preview changes without publishingregistry(optional):verdaccio- Publish to local Verdaccio registry (http://localhost:4873/)npm- Publish to npmjs.org (default)skip_push(optional): Passskip_pushto skip git push operations (commits/tags created locally only)Usage Examples
Verdaccio Support
The release task now includes full support for local testing with Verdaccio:
npm install -g verdaccioverdaccionpm adduser --registry http://localhost:4873/rake release[16.2.0,false,verdaccio]When using Verdaccio mode:
Release Process Flow
The task executes these steps:
lib/react_on_rails/version.rbreact-on-rails-prodependency to exact versionv{version}skip_pushis set)react-on-railsto NPM/Verdaccioreact-on-rails-proto NPM/Verdaccioreact_on_railsto RubyGems (unless Verdaccio mode)Benefits
✅ No external
release-itdependency✅ Both NPM packages properly released
✅ Version sync enforced across all packages
✅ Simpler, pure Ruby + Yarn solution
✅ Single command releases everything
✅ Local testing support with Verdaccio
✅ Flexible workflow options (dry-run, skip-push)
✅ Parameter validation prevents typos
Test Plan
rake release[16.1.2,true]🤖 Generated with Claude Code
This change is