-
Notifications
You must be signed in to change notification settings - Fork 1
Add CONTRIBUTING.md, enhanced CHANGELOG.md, and release rake task #24
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
- Add CONTRIBUTING.md with guidelines for reporting issues, PRs, setup, testing, and linting (consistent with Shakapacker/React on Rails) - Update CHANGELOG.md to follow keepachangelog format with proper structure and initial release notes - Add rakelib/release.rake for gem releases with support for version bumping (patch/minor/major or explicit), dry-run mode, and retry logic - Update gemspec to use UberTask::VERSION constant instead of hardcoded version string Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
WalkthroughInfrastructure and documentation enhancements including a restructured changelog with proper versioning sections, new contributor guidelines, automated gem release workflow via Rake task, and dynamic version referencing in the gemspec file. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@CONTRIBUTING.md`:
- Around line 101-109: The CONTRIBUTING.md section claims lefthook configuration
exists but the repo lacks a .lefthook.yml (lefthook.yml) file despite lefthook
being in the Gemfile; either add the lefthook config file to the repository
(create .lefthook.yml with the project's pre-commit/pre-push hook definitions)
or update the CONTRIBUTING.md text to remove/clarify the lefthook instructions
and note that hooks are opt-in and require manual config; update references to
lefthook in CONTRIBUTING.md and ensure the Gemfile or README mentions how to
obtain/configure the missing .lefthook.yml if you choose not to commit it.
In `@rakelib/release.rake`:
- Around line 117-119: The otp handling currently builds otp_option from
ENV['RUBYGEMS_OTP'] and injects it into the gem release command (otp_option /
sh_with_retry and the "gem release" invocation), which exposes the OTP on the
command line; remove the --otp flag usage and instead ensure GEM_HOST_OTP_CODE
is used: if ENV['RUBYGEMS_OTP'] is present, set ENV['GEM_HOST_OTP_CODE'] =
ENV['RUBYGEMS_OTP'] (or prefer reading ENV['GEM_HOST_OTP_CODE'] directly) and
call sh_with_retry("gem release") without the --otp argument so the OTP is
provided via environment rather than process args.
🧹 Nitpick comments (6)
CHANGELOG.md (1)
11-17: Missing link definitions for version references.The bracketed version headers
[Unreleased]and[0.1.0]use Markdown reference-style link syntax, but the link definitions are missing. Per keepachangelog convention, these should link to GitHub compare/tag URLs.📝 Add link definitions at the bottom of the file
- Colorized console output for task progress + +[Unreleased]: https://github.com/shakacode/uber_task/compare/v0.1.0...HEAD +[0.1.0]: https://github.com/shakacode/uber_task/releases/tag/v0.1.0rakelib/release.rake (5)
27-50: Consider extracting helper methods outside the task block.Defining methods inside the task block works but is unconventional. These methods are redefined on each task invocation and aren't easily testable or reusable.
♻️ Move helpers to module scope
# frozen_string_literal: true +module ReleaseHelpers + def self.log(message, force: false) + puts message if ENV['VERBOSE'] == '1' || force + end + + def self.sh_with_retry(command, max_retries: 3) + retries = 0 + max_retries = ENV.fetch('GEM_RELEASE_MAX_RETRIES', max_retries).to_i + + loop do + log "Running: #{command}" + success = system(command) + return true if success + + retries += 1 + raise "Command failed after #{max_retries} attempts: #{command}" if retries >= max_retries + + puts "Command failed (attempt #{retries}/#{max_retries}). Retrying..." + if command.include?('gem push') || command.include?('gem release') + puts 'Enter new RubyGems OTP if needed:' + end + end + end +end + desc <<~DESC
85-89: Redundantrequire_relativebeforeload.The
require_relativeon line 86 is unnecessary sinceloadon line 88 unconditionally re-evaluates the file. Theloadalone is sufficient for reloading the version after the bump.♻️ Simplify version reload
# Get the new version - require_relative '../lib/uber_task/version' # Force reload of version file load File.join(gem_root, 'lib', 'uber_task', 'version.rb') new_version = UberTask::VERSION
99-101: Consider explicitly staging only the expected files.
git add -Astages all changes. While the clean git check provides safety, explicitly adding only the version file and Gemfile.lock would be more predictable and self-documenting.♻️ Explicit file staging
unless dry_run - sh_with_retry("git add -A && git commit -m 'Release v#{new_version}'") + sh_with_retry("git add lib/uber_task/version.rb Gemfile.lock && git commit -m 'Release v#{new_version}'") sh_with_retry("git tag -a v#{new_version} -m 'Version #{new_version}'") end
52-56: Consider validatinggem-releaseis installed before proceeding.The task documents the gem-release dependency but doesn't verify it's available. Adding an early check would provide a clearer error message.
♻️ Add dependency check
+ # Check for gem-release dependency + unless system('gem list -i gem-release > /dev/null 2>&1') + raise 'ERROR: gem-release is required. Install it with: gem install gem-release' + end + # Check for uncommitted changes unless `git status --porcelain`.strip.empty? raise 'ERROR: Uncommitted changes found. Commit or stash them first.' end
126-129: CHANGELOG update recommended before release, not after.The post-release guidance suggests updating CHANGELOG.md after the release. However, best practice is to update the changelog before tagging so release notes are included in the tagged version. Consider either:
- Prompting for CHANGELOG updates before committing/tagging, or
- Documenting that maintainers should update CHANGELOG before running the release task.
| ## Git Hooks (Optional) | ||
|
|
||
| This project includes configuration for git hooks via `lefthook`. They are **opt-in for contributors**. | ||
|
|
||
| To enable pre-commit hooks locally: | ||
|
|
||
| ```bash | ||
| lefthook install | ||
| ``` |
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
🏁 Script executed:
#!/bin/bash
# Check for lefthook configuration files in common locations
echo "=== Searching for lefthook configuration files ==="
fd -type f "lefthook" 2>/dev/null | head -20
# Also try common naming patterns
echo ""
echo "=== Checking for .lefthook.yml or lefthook.yml ==="
fd -type f -name "*lefthook*" 2>/dev/null | head -20
# Check for lefthook in dependency files
echo ""
echo "=== Checking for lefthook in Gemfile ==="
if [ -f "Gemfile" ]; then
rg -n "lefthook" Gemfile 2>/dev/null || echo "Not found in Gemfile"
else
echo "Gemfile not found"
fi
echo ""
echo "=== Checking for lefthook in package.json ==="
if [ -f "package.json" ]; then
rg -n "lefthook" package.json 2>/dev/null || echo "Not found in package.json"
else
echo "package.json not found"
fi
# Check for any git hooks configuration
echo ""
echo "=== Checking for .husky, .git/hooks, or other hook configs ==="
fd -type d -name ".husky" 2>/dev/null || echo "No .husky directory"
fd -type f "husky\|pre-commit\|git-hooks" 2>/dev/null | head -10 || echo "No other hook configs found"Repository: shakacode/uber_task
Length of output: 393
Lefthook configuration file is missing from the repository.
While the Gemfile declares lefthook as a dependency (line 6), there is no .lefthook.yml or lefthook.yml configuration file present in the repository. The documentation claims "This project includes configuration for git hooks via lefthook", but without the configuration file, contributors running lefthook install will not have any hooks configured. Either add the lefthook configuration file to the repository or remove/clarify this section in CONTRIBUTING.md to reflect the actual state of git hooks setup.
🤖 Prompt for AI Agents
In `@CONTRIBUTING.md` around lines 101 - 109, The CONTRIBUTING.md section claims
lefthook configuration exists but the repo lacks a .lefthook.yml (lefthook.yml)
file despite lefthook being in the Gemfile; either add the lefthook config file
to the repository (create .lefthook.yml with the project's pre-commit/pre-push
hook definitions) or update the CONTRIBUTING.md text to remove/clarify the
lefthook instructions and note that hooks are opt-in and require manual config;
update references to lefthook in CONTRIBUTING.md and ensure the Gemfile or
README mentions how to obtain/configure the missing .lefthook.yml if you choose
not to commit it.
| unless dry_run | ||
| otp_option = ENV['RUBYGEMS_OTP'] ? "--otp #{ENV['RUBYGEMS_OTP']}" : '' | ||
| sh_with_retry("gem release #{otp_option}") |
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
🌐 Web query:
gem-release Ruby gem OTP authentication environment variable
💡 Result:
Use the GEM_HOST_OTP_CODE environment variable (or the --otp flag) to supply a RubyGems OTP when pushing/releases. Example:
GEM_HOST_OTP_CODE=$(ykman oath accounts code -s rubygems.org) gem push mygem-0.1.0.gem
Sources: RubyGems command docs and Gem utilities (env var name) and example usage. [1][2][3]
References
- RubyGems guides: Using OTP / command reference. [1]
- Ruby stdlib docs (Gem::GemcutterUtilities add_otp_option shows GEM_HOST_OTP_CODE). [2]
- Example (YubiKey + env var) blog showing GEM_HOST_OTP_CODE usage. [3]
🌐 Web query:
gem-release gem documentation OTP handling rubygems
💡 Result:
Short answer: gem-release itself doesn’t implement OTP handling — it calls the normal RubyGems push/signin flow, so OTP (2FA) is handled by the rubygems CLI. To release a gem when your account has OTP MFA you must either provide the OTP to the rubygems command (gem push / gem signin) or use an alternative publishing flow (API key / CI tokens / trusted-publisher action). [1][2][5]
Practical options and notes
- Provide OTP on the command line: recent rubygems supports --otp or the GEM_HOST_OTP_CODE environment variable: e.g. gem push mygem-0.1.0.gem --otp 123456 or GEM_HOST_OTP_CODE=$(oathtool ...) gem push .... Rubygems documents using OTP in the CLI. [1]
- If your local gem/rubygems is old it may not accept --otp or prompt correctly (known pre-3.1 / pre-3.0 issues). Upgrade rubygems (gem update --system) or use a newer Ruby that ships a modern gem CLI. [1][8]
- Bundler / rake release: older bundler/release tasks can “freeze” during interactive OTP prompts. Workarounds: type the OTP when prompted, upgrade Bundler to a version that handles prompts (≥ 2.0.2), or fetch and store the API key manually (curl with OTP header) before releasing. [3]
- Non-interactive CI publishing: use Rubygems’ trusted-publishing / release-gem GitHub Action or store an API key / GitHub OIDC/token in CI to avoid interactive OTP prompts. Also you can configure a hardware TOTP (YubiKey) and script OTP generation. [5][4]
- Enforcing MFA for a gem: gem owners can mark a gem as requiring MFA for subsequent releases (metadata.rubygems_mfa_required). If a gem requires MFA, owners without MFA cannot push. [6]
If you want, I can:
- show exact commands for your environment (local CLI, Bundler rake release, or GitHub Actions), or
- check your rubygems/gem-release versions and recommend specific upgrades.
Use GEM_HOST_OTP_CODE environment variable instead of passing OTP via --otp flag.
The current approach passes OTP as a command-line argument (--otp #{ENV['RUBYGEMS_OTP']}), which is visible in process lists on shared systems. RubyGems supports the more secure GEM_HOST_OTP_CODE environment variable, which is not visible in process listings. Either rely on GEM_HOST_OTP_CODE directly (which gem push reads natively) or avoid the --otp flag entirely when OTP is set via environment variable.
🤖 Prompt for AI Agents
In `@rakelib/release.rake` around lines 117 - 119, The otp handling currently
builds otp_option from ENV['RUBYGEMS_OTP'] and injects it into the gem release
command (otp_option / sh_with_retry and the "gem release" invocation), which
exposes the OTP on the command line; remove the --otp flag usage and instead
ensure GEM_HOST_OTP_CODE is used: if ENV['RUBYGEMS_OTP'] is present, set
ENV['GEM_HOST_OTP_CODE'] = ENV['RUBYGEMS_OTP'] (or prefer reading
ENV['GEM_HOST_OTP_CODE'] directly) and call sh_with_retry("gem release") without
the --otp argument so the OTP is provided via environment rather than process
args.
|
@claude review this PR. |
|
Claude finished @justin808's task in 1m 0s —— View job Code Review Complete ✅I've reviewed all four changed files in this PR. Overall, this is a solid addition of project infrastructure with well-documented guidelines and automation. Below is my detailed feedback organized by file. 🟢 CONTRIBUTING.md - Excellent with one minor noteStrengths:
Note:
Minor suggestions:
🟡 CHANGELOG.md - Good format, missing link definitionsStrengths:
Issue - Missing link definitions: Per keepachangelog convention, add these at the end: [Unreleased]: https://github.com/shakacode/uber_task/compare/v0.1.0...HEAD
[0.1.0]: https://github.com/shakacode/uber_task/releases/tag/v0.1.0🟡 rakelib/release.rake - Well-designed with security concernStrengths:
Critical Security Issue - OTP exposure in process arguments: Lines 117-119: The OTP is passed via command-line argument ( Current code: otp_option = ENV['RUBYGEMS_OTP'] ? "--otp #{ENV['RUBYGEMS_OTP']}" : ''
sh_with_retry("gem release #{otp_option}")Recommended fix: # Set the environment variable that gem push/release reads
ENV['GEM_HOST_OTP_CODE'] = ENV['RUBYGEMS_OTP'] if ENV['RUBYGEMS_OTP']
sh_with_retry("gem release") # No --otp flag neededThis keeps the OTP out of the process argument list while maintaining the same functionality. Additional suggestions (optional improvements):
🟢 uber_task.gemspec - Perfect improvementStrengths:
No issues found. This is a clean refactor that improves maintainability. SummaryApprove with minor fixes recommended: Must fix:
Optional improvements:
The PR adds valuable infrastructure and documentation. The release automation is particularly well-designed with good safety checks and retry logic. Once the OTP security concern is addressed, this will be ready to merge. Test plan status:
|
Summary
UberTask::VERSIONconstant instead of hardcoded version stringTest plan
bundle exec rubocoppassesbundle exec rspecpasses (40 examples, 0 failures)🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.