-
-
Notifications
You must be signed in to change notification settings - Fork 62
Add simplified gem release process #187
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
Conversation
This commit adds a streamlined release process for the cypress-on-rails gem, removing the npm package release steps that were present in the react_on_rails version. Key changes: - Add lib/tasks/release.rake with gem-only release automation - Add docs/RELEASE.md with comprehensive release documentation - Simplify process by removing release-it and npm publishing steps - Keep essential features: version bumping, git tagging, and gem publishing The release task handles: - Checking for uncommitted changes - Pulling latest changes - Bumping version using gem-release - Publishing to RubyGems with OTP support - Providing post-release instructions for CHANGELOG updates 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Warning Rate limit exceeded@justin808 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 26 minutes and 21 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 (3)
WalkthroughAdds a release process document and a new Rake task to automate gem releases, including preflight checks, optional version bumping, rebase pull, release execution (with dry-run support), OTP guidance, and post-release changelog/commit instructions. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant Rake as Rake Task: release
participant Git as Git
participant Gem as gem CLI
participant RG as RubyGems
Dev->>Rake: rake release[gem_version,dry_run]
Rake->>Git: status (check for uncommitted changes)
alt Uncommitted changes
Rake-->>Dev: Abort with error
else Clean
Rake->>Git: pull --rebase (at gem root)
opt gem_version provided
Rake->>Gem: bump --no-commit <version>
end
alt dry_run
Rake-->>Dev: Print simulated steps and post-release instructions
else real release
Rake->>Gem: release
note over Rake,Gem: Prompt for OTP if required
Gem->>RG: Push gem
RG-->>Gem: Publish result
Gem-->>Rake: Done
Rake-->>Dev: Print CHANGELOG/commit/push follow-ups
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 for PR 187: Add simplified gem release process Overview: Code Quality and Best Practices Strengths:
Issues:
Potential Bugs or Issues
Security Concerns
Test Coverage Missing:
Recommendations:
Documentation Quality Strengths:
Minor issues:
Additional Recommendations
Summary Severity Breakdown:
Recommendation: Approve with changes This PR provides valuable release automation, but should address the shell injection vulnerability and missing dependency before merging. Action Items:
Great work on the comprehensive documentation! |
Specify that pre-release versions must use dot notation (e.g., 2.0.0.beta.1) rather than dash notation (e.g., 2.0.0-beta.1) as required by RubyGems. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
The update_changelog task doesn't exist in this repository - it was copied from react_on_rails but never implemented here. Update both the release.rake and RELEASE.md to indicate manual CHANGELOG updates. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
PR Review: Add simplified gem release processSummaryThis PR introduces a rake task and documentation to streamline the gem release process. Overall, the implementation is solid and provides good value. Below are my findings organized by category. Code Quality & Best PracticesGood:
Suggestions:
Potential Bugs
Performance ConsiderationsNo significant performance concerns. The task is designed for infrequent use and operations are appropriately sequential. Security Concerns
Test CoverageMissing:
Recommendations:
Documentation QualityGood:
Issues:
Additional Observations
Recommendations SummaryMust Fix:
Should Fix:
Nice to Have:
ConclusionThis is a valuable addition that will streamline the release process. The code is generally well-written and the documentation is thorough. The primary blocker is the missing update_changelog task. Once that is addressed and tests are added, this will be ready to merge. Overall Assessment: Approve with changes requested |
Pull Request ReviewThank you for this contribution! I have reviewed PR #187 and have the following feedback: ✅ Strengths
🔍 Code Quality Observationslib/tasks/release.rake Line 2: The frozen_string_literal comment is good practice ✓ Lines 24-26: Good safety check for uncommitted changes. However, consider adding a check for unpushed commits as well. Lines 33-34: The git pull --rebase is good, but consider checking if already up-to-date first to avoid unnecessary operations. Line 35: The gem bump command looks correct. Consider capturing the new version number to display it. Line 38: The error handling message is helpful, but you might want to capture and handle gem-release errors more gracefully. docs/RELEASE.md Line 72: Good use of semantic versioning reference. Pre-release version format: You mention using dot notation instead of dashes. This is correct for RubyGems, but consider adding a note explaining why. Line 86: Consider mentioning that gem-release needs to be installed globally, not just in the bundle.
|
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/tasks/release.rake (1)
82-86
: Inconsistency: Task does not create commits or tags.According to the implementation, the task uses
gem bump --no-commit
, meaning it does NOT create commits or tags itself. However, the documentation claims it does (lines 82-86 in docs/RELEASE.md). Thegem release
command likely handles the commit, tag, and push operations, but this should be verified.The documentation states:
- "Creates a git commit with the version bump" (line 82)
- "Creates a git tag for the new version" (line 83)
- "Pushes the commit and tag to GitHub" (line 84)
But the task uses
--no-commit
flag, which suggests these operations happen in thegem release
command instead. Please verify the actual behavior and update the documentation accordingly to accurately reflect which command performs which operations.
🧹 Nitpick comments (1)
lib/tasks/release.rake (1)
35-35
: Complex inline string interpolation reduces readability.The nested conditional and string interpolation on line 35 is difficult to parse and maintain.
Apply this diff to improve clarity:
- sh_in_dir(gem_root, "gem bump --no-commit #{%(--version #{gem_version}) unless gem_version.strip.empty?}") + version_arg = gem_version.strip.empty? ? "" : "--version #{gem_version}" + sh_in_dir(gem_root, "gem bump --no-commit #{version_arg}".strip)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/RELEASE.md
(1 hunks)lib/tasks/release.rake
(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). (4)
- GitHub Check: claude-review
- GitHub Check: rails_8
- GitHub Check: rails_6_1
- GitHub Check: rails_7_2
🔇 Additional comments (8)
docs/RELEASE.md (3)
20-26
: Document is clear and helpful.The preflight check instructions are clear and align well with the implementation's uncommitted changes check.
66-74
: Version numbering guidance is accurate.The Semantic Versioning guidance is clear and helpful, including the important note about pre-release version format using dots instead of dashes for RubyGems compatibility.
81-81
: No changes required: version file path is accurate. The filelib/cypress_on_rails/version.rb
exists in the repository.lib/tasks/release.rake (5)
25-27
: Preflight check implementation is correct.The uncommitted changes check properly guards against releasing with dirty working directory state, matching the documentation.
29-31
: Argument parsing is straightforward.The task correctly parses the dry_run boolean and gem_version arguments with sensible defaults.
38-39
: OTP guidance is helpful.The reminder to carefully enter the OTP and instructions for manual retry if needed are user-friendly touches that will help prevent common release errors.
41-48
: Post-release instructions are clear.The heredoc message provides clear next steps for updating the changelog, which aligns with the documented process.
33-35
: gem-release commands are valid
gem bump --no-commit
correctly suppresses the default commit, and invokinggem release
thereafter aligns with the gem-release documentation. No changes needed.
1. Install the `gem-release` gem globally: | ||
```bash | ||
gem install gem-release | ||
``` |
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.
Verify gem-release installation method.
The documentation suggests installing gem-release
globally, but the task description at line 7 of lib/tasks/release.rake
states it "is installed via bundle install
". This inconsistency could confuse users.
Consider updating the documentation to clarify whether the gem should be installed globally or via Bundler, or document both approaches.
-1. Install the `gem-release` gem globally:
+1. Install the `gem-release` gem (either globally or via your Gemfile):
```bash
+ # Option 1: Global installation
gem install gem-release
+
+ # Option 2: Add to your Gemfile and run bundle install
+ # gem 'gem-release'
```
🤖 Prompt for AI Agents
In docs/RELEASE.md around lines 7 to 10, the instructions conflict with
lib/tasks/release.rake which says gem-release is installed via bundle; update
the docs to be explicit about both supported installation methods (global vs
Bundler) and show the recommended approach: either a single-line note to run
`gem install gem-release` for global use or, preferably, add gem 'gem-release'
to the project's Gemfile and run `bundle install`; ensure the wording clarifies
which method the release task expects (Bundler by default) and give both options
so readers are not confused.
def sh_in_dir(dir, command) | ||
puts "Running in #{dir}: #{command}" | ||
system("cd #{dir} && #{command}") || raise("Command failed: #{command}") | ||
end |
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.
Critical: Command injection vulnerability.
The sh_in_dir
method is vulnerable to command injection because it directly interpolates dir
and command
into a shell string without sanitization. A malicious directory path or command could escape the intended context.
Apply this diff to use safer command execution:
def sh_in_dir(dir, command)
puts "Running in #{dir}: #{command}"
- system("cd #{dir} && #{command}") || raise("Command failed: #{command}")
+ Dir.chdir(dir) do
+ system(command) || raise("Command failed: #{command}")
+ end
end
Alternatively, for even better safety, consider using Bundler.with_unbundled_env
and splitting commands into arrays when possible.
📝 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.
def sh_in_dir(dir, command) | |
puts "Running in #{dir}: #{command}" | |
system("cd #{dir} && #{command}") || raise("Command failed: #{command}") | |
end | |
def sh_in_dir(dir, command) | |
puts "Running in #{dir}: #{command}" | |
Dir.chdir(dir) do | |
system(command) || raise("Command failed: #{command}") | |
end | |
end |
🤖 Prompt for AI Agents
In lib/tasks/release.rake around lines 15-18, the sh_in_dir method interpolates
dir and command into a single shell string which allows command injection;
replace the shell interpolation with a safe approach: change to using
Dir.chdir(dir) { success = system(*Shellwords.split(command)) } (require
'shellwords') or use Open3.capture3/system with array arguments and a chdir
option (e.g., Open3.capture3(command_array, chdir: dir)) so no untrusted string
is passed to the shell; also consider wrapping execution with
Bundler.with_unbundled_env where appropriate and raise with a clear error when
the command returns false.
Copied and adapted the update_changelog task from react_on_rails to automatically update CHANGELOG.md with new version headers and links. Key features: - Automatically detects latest git tag - Inserts new version header with release date under [Unreleased] - Updates version comparison links at bottom of CHANGELOG - Handles version tags with or without 'v' prefix - Provides clear next steps for manual edits Also updated release.rake and RELEASE.md to reference the new task. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Pull Request Review - PR #187: Add simplified gem release processOverviewThis PR adds a comprehensive release automation system for the cypress-on-rails gem. The implementation is well-structured and addresses a real need for streamlining the release process. StrengthsCode Quality
Best Practices
Issues and Recommendations1. Critical: Missing gem-release dependencyThe release task depends on the gem-release gem but it is not declared in the gemspec or Gemfile. Issue: Users will get command failures when running gem bump or gem release (lib/tasks/release.rake:34-38) Fix: Add to cypress-on-rails.gemspec: s.add_development_dependency 'gem-release' 2. Bug: Changelog regex does not handle pre-release versions correctlyThe regex pattern in update_changelog.rake:44 may fail with pre-release versions containing dots. Issue: The pattern (?:.\w+)? only matches single alphanumeric characters after the dot. For 2.0.0.beta.1, it will not match .1 correctly. Fix: Change to use a more flexible pattern with asterisk instead of question mark to match zero or more occurrences. 3. Security: Command injection vulnerabilityThe sh_in_dir method does not sanitize inputs (lib/tasks/release.rake:16-17). Risk: If gem_version contains shell metacharacters, it could lead to command injection. Fix: Validate the gem_version parameter before interpolation with a regex that ensures it matches semantic versioning format. 4. Race Condition: Git pull before version bumpThe task pulls latest changes (lib/tasks/release.rake:33) but does not verify the working directory is still clean afterward. Recommendation: Add another check after git pull to ensure no conflicts or uncommitted changes were introduced. 5. Documentation: Incomplete troubleshootingdocs/RELEASE.md does not cover what to do if gem bump fails or if tags already exist locally but not remotely. Recommendation: Add troubleshooting sections for tag conflicts, network failures during push, and RubyGems rate limiting. Test CoverageMissing: No automated tests for the rake tasks. Recommendation: Add specs for release.rake and update_changelog.rake with mocked git/gem commands and fixture CHANGELOG files. Test edge cases like empty version, malformed CHANGELOG, and missing tags. Security AssessmentLow Risk overall, but address the command injection risk in version parameter. Summary ChecklistMust Fix (Blocking)
Should Fix (Non-blocking but important)
Nice to Have
Final VerdictRecommendation: Approve with changes requested The PR provides solid infrastructure for release automation. Address the gem-release dependency and version validation issues before merging. The other suggestions can be addressed in follow-up PRs. Great work simplifying the release process! Review generated with assistance from Claude Code |
Summary
lib/tasks/release.rake
with gem-only release automationdocs/RELEASE.md
with comprehensive release documentationKey Changes
New Release Task (
lib/tasks/release.rake
)gem-release
gemNew Documentation (
docs/RELEASE.md
)Usage
Test Plan
rake release[1.19.0,true]
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation