Skip to content

Conversation

@ihabadham
Copy link
Collaborator

@ihabadham ihabadham commented Dec 30, 2025

Summary

  • Removes the v prefix from git tag creation in the release script to restore the historical convention used from version 2.0.0 through 16.1.2
  • Updates documentation to reflect the correct tagging convention
  • Adds notes about the historical v16.2.0.beta.x tags that were created with v prefix

Background

PR #1856 unintentionally introduced a v prefix to git tags when replacing release-it with manual git commands. The release-it tool auto-detected the existing tag format (no v prefix), but the manual implementation explicitly added v#{version}.

This broke 9+ years of consistent tagging (222 tags without v prefix vs 30 tags with v prefix) and caused issues like broken CHANGELOG links (fixed in PR #2263).

Changes

File Change
rakelib/release.rake Remove v prefix from tag_name variable
docs/contributor-info/releasing.md Update tag format examples
.claude/commands/update-changelog.md Update tag convention documentation

Test plan

  • Verify release script creates tags without v prefix (dry run: rake release[16.3.0,true])
  • Confirm documentation accurately describes the tag convention

Fixes #2267

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Documentation
    • Updated release and contributor docs to use unprefixed version tags (e.g., 16.2.1) in examples and commands.
    • Added notes clarifying historical beta/rc tags may include a temporary "v" prefix and instructing to strip any historical "v" when adding entries to the changelog.
    • Adjusted examples and guidance for creating, deleting, and displaying latest tags to the non‑prefixed convention.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 30, 2025

Walkthrough

The PR removes the leading "v" prefix from git tag examples and tag creation in release tooling and docs, updates changelog guidance to use unprefixed versions, and adds notes about historical v-prefixed tags that must be stripped when composing changelog links.

Changes

Cohort / File(s) Summary
Documentation
\.claude/commands/update-changelog.md, docs/contributor-info/releasing.md
Updated examples and instructions to use tag names without a leading v; added note blocks documenting historical v-prefixed tags and advising stripping them for changelog/compare links.
Release tooling
rakelib/release.rake, react_on_rails/rakelib/update_changelog.rake
Construct tag names without the v prefix when creating/pushing tags; adjusted comments to reflect historical v-prefix and current unprefixed convention.

Sequence Diagram(s)

(omitted — changes are documentation and small release-tooling adjustments without new multi-component control flow)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

Possibly related PRs

Suggested reviewers

  • justin808
  • Romex91

Poem

🐰
I hopped through tags both old and new,
Removed a v to keep the view true,
Docs refreshed and rakefile light,
Legacy v’s tucked out of sight,
Happy releases — hop, hooray! 🎉

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and accurately summarizes the main change: removing the v prefix from git tags to restore historical convention.
Linked Issues check ✅ Passed The pull request successfully addresses all coding requirements from issue #2267: removes v prefix from tag creation in rakelib/release.rake, updates documentation in docs/contributor-info/releasing.md and .claude/commands/update-changelog.md, and documents historical v-prefixed beta tags.
Out of Scope Changes check ✅ Passed All changes are directly related to the stated objectives: tag naming convention updates in code and documentation. No unrelated modifications detected.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch ihabadham/fix/remove-v-prefix-from-git-tags

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d2e2bc7 and 8c27aed.

📒 Files selected for processing (4)
  • .claude/commands/update-changelog.md
  • docs/contributor-info/releasing.md
  • rakelib/release.rake
  • react_on_rails/rakelib/update_changelog.rake
✅ Files skipped from review due to trivial changes (1)
  • react_on_rails/rakelib/update_changelog.rake
🚧 Files skipped from review as they are similar to previous changes (2)
  • docs/contributor-info/releasing.md
  • .claude/commands/update-changelog.md
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T18:57:59.314Z
Learning: Applies to /{CHANGELOG.md,CHANGELOG_PRO.md} : Use format `[PR 1818](https://github.com/shakacode/react_on_rails/pull/1818) by [username](https://github.com/username)` in changelog entries (no hash in PR number)
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T18:57:59.314Z
Learning: Applies to /{CHANGELOG.md,CHANGELOG_PRO.md} : DO NOT add changelog entries for: linting, formatting, refactoring, tests, or documentation fixes - only user-visible changes
📚 Learning: 2025-12-19T18:57:59.314Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T18:57:59.314Z
Learning: Applies to /CHANGELOG.md : Update `/CHANGELOG.md` for user-visible changes in the open-source react_on_rails gem and npm package (features, bug fixes, breaking changes, deprecations, performance improvements)

Applied to files:

  • rakelib/release.rake
📚 Learning: 2025-12-19T18:57:59.314Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T18:57:59.314Z
Learning: Applies to /CHANGELOG_PRO.md : Update `/CHANGELOG_PRO.md` for user-visible changes in the Pro-only react_on_rails_pro gem and npm packages

Applied to files:

  • rakelib/release.rake
📚 Learning: 2025-12-19T18:57:59.314Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T18:57:59.314Z
Learning: Applies to /{CHANGELOG.md,CHANGELOG_PRO.md} : Use format `[PR 1818](https://github.com/shakacode/react_on_rails/pull/1818) by [username](https://github.com/username)` in changelog entries (no hash in PR number)

Applied to 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). (9)
  • GitHub Check: dummy-app-integration-tests (3.4, 22, latest)
  • GitHub Check: build
  • GitHub Check: examples (3.4, latest)
  • GitHub Check: build-dummy-app-webpack-test-bundles
  • GitHub Check: rspec-package-tests (3.4, latest)
  • GitHub Check: build-dummy-app-webpack-test-bundles
  • GitHub Check: precompile-check
  • GitHub Check: pro-lint-js-and-ruby
  • GitHub Check: claude-review
🔇 Additional comments (2)
rakelib/release.rake (2)

249-251: LGTM! Tag format correctly restored to historical convention.

The removal of the 'v' prefix from the tag name aligns with the stated goal of restoring consistency with tags created from versions 2.0.0 through 16.1.2. The explanatory comments provide helpful context for future maintainers.


252-257: Tag handling logic works correctly with the unprefixed format.

The tag existence check (git rev-parse) and creation (git tag) commands function properly with unprefixed tag names. The update_changelog task explicitly handles both prefixed (historical beta tags like v16.2.0.beta.0) and unprefixed tags by stripping the prefix when needed. No other scripts in the repository expect hardcoded 'v' prefixes—the implementation correctly restores the 9+ year historical convention of unprefixed tags.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@claude
Copy link

claude bot commented Dec 30, 2025

Code Review Feedback

✅ Overall Assessment

This is a well-executed fix that restores consistency to the git tag naming convention. The change is minimal, well-documented, and addresses the root cause identified in issue #2267.


🎯 Strengths

  1. Correct Historical Analysis: The PR correctly identifies that 222 tags used no v prefix (2.0.0-16.1.2) vs only 30 tags with v prefix (16.2.0 betas). Restoring the historical convention is the right choice.

  2. Minimal Code Change: Only 3 substantive changes in release.rake:

    • Removes v prefix from tag_name
    • Adds clear inline comment explaining the convention
    • Maintains backward compatibility (tag existence check still works)
  3. Comprehensive Documentation Updates:

    • .claude/commands/update-changelog.md: Updated with accurate examples and historical context
    • docs/contributor-info/releasing.md: Clarified tag format in release process
    • All updates acknowledge the historical anomaly (v16.2.0.beta.x tags)
  4. No Breaking Changes: This only affects future tags. Existing tags remain unchanged, and the script handles both formats gracefully.


🔍 Code Quality

rakelib/release.rake (lines 248-251)

# Create git tag (skip if it already exists)
# Note: Tags use version without 'v' prefix (e.g., '16.2.0' not 'v16.2.0')
# to match the historical convention used from version 2.0.0 through 16.1.2
tag_name = actual_gem_version

Good:

  • Clear, explanatory comment
  • Uses existing actual_gem_version variable (no new dependencies)
  • Tag existence check (git rev-parse #{tag_name}) works with both formats

🧪 Test Plan Recommendations

The PR mentions a test plan but doesn't show verification. I recommend testing:

# 1. Dry run to verify tag name format
rake release[16.3.0,true]
# Expected: Should show tag creation as "git tag 16.3.0" (no 'v')

# 2. Verify tag existence check handles both formats
git tag 16.2.1  # Create test tag without 'v'
rake release[16.2.1,true]
# Expected: Should detect existing tag and skip creation

# 3. Verify rollback scenario (from docs/releasing.md:267)
git tag -d 16.2.1  # Delete test tag
# Expected: Should work (no 'v' prefix assumed)

📝 Minor Suggestions

  1. CHANGELOG Entry: This PR should have an entry in CHANGELOG.md under ### [Unreleased]:

    #### Fixed
    [PR 2268](https://github.com/shakacode/react_on_rails/pull/2268): Removed `v` prefix from git tags to restore historical convention (no user-facing impact). by [ihabadham](https://github.com/ihabadham)

    Rationale: While this doesn't affect end users, it affects contributors and the release process, making it worth documenting.

  2. Documentation Precision (.claude/commands/update-changelog.md):
    The historical note appears 3 times with slightly different wording. Consider consolidating to a single footnote-style reference for consistency:

    > **Historical Note**: Tags v16.2.0.beta.0 through v16.2.0.rc.0 have a `v` prefix due to a temporary convention change in PR #1856. The standard convention (restored in PR #2268) uses no `v` prefix.
  3. Rollback Documentation: The rollback example in docs/contributor-info/releasing.md:267 is now correct, but consider adding a note:

    - Delete the tag: `git tag -d X.Y.Z && git push origin :X.Y.Z`
    # Note: For historical v16.2.0.beta.x tags, use: git tag -d vX.Y.Z

🔒 Security & Performance

  • ✅ No security concerns (changes only affect release process)
  • ✅ No performance impact (single string assignment)

🎬 Final Recommendation

APPROVE

This PR successfully:

The minor suggestions above are optional improvements. The PR is ready to merge as-is.


📚 References

@claude
Copy link

claude bot commented Dec 30, 2025

Code Review

Summary

This PR correctly addresses the git tag naming inconsistency by removing the v prefix to restore the historical convention used from version 2.0.0 through 16.1.2. The changes are well-documented and align with the project's 9+ year tagging history.

✅ Strengths

  1. Clear Problem Statement: The PR and related issue Inconsistent git tag naming convention: 'v' prefix added in new release script #2267 provide excellent context about why this change is needed
  2. Comprehensive Documentation Updates: All three affected files are updated consistently:
    • rakelib/release.rake - the actual implementation
    • docs/contributor-info/releasing.md - contributor documentation
    • .claude/commands/update-changelog.md - changelog management guide
  3. Historical Context: Good documentation of the temporary v prefix convention (v16.2.0.beta.0 through v16.2.0.rc.0)
  4. Inline Comments: Added helpful comments in release.rake explaining the convention choice

📋 Observations

  1. CI Failure: The markdown-link-check workflow is failing. This might be unrelated to your changes, but please investigate to ensure it's not caused by the documentation updates.

  2. Testing Recommendation: Since this affects the release process, I recommend:

    • Testing the dry-run mode: rake release[16.3.0,true] to verify the tag name format
    • Double-checking that the tag creation logic works correctly without the v prefix
  3. Consistency Check: The changes look consistent across all files. The pattern vX.Y.Z has been properly updated to X.Y.Z (no v prefix) in both examples and documentation.

🔍 Code Quality

rakelib/release.rake:249-251

# Note: Tags use version without 'v' prefix (e.g., '16.2.0' not 'v16.2.0')
# to match the historical convention used from version 2.0.0 through 16.1.2
tag_name = actual_gem_version

✅ Clean implementation with good inline documentation explaining the reasoning.

Documentation Files
✅ All documentation updates are clear and include notes about the historical v16.2.0.beta.x tags to prevent future confusion.

🎯 Recommendations

  1. Investigate CI Failure: Check the markdown-link-check failure before merging
  2. Test Dry Run: Run rake release[16.3.0,true] to verify the tag format
  3. Consider CHANGELOG Update: Does this change itself need a CHANGELOG entry? It's a developer-facing change to the release process, which might warrant documentation.

Security & Performance

✅ No security concerns
✅ No performance impact

Test Coverage

⚠️ This change is to the release script, which is typically tested manually. The dry-run mode should provide adequate verification.

Overall Assessment

Excellent work! This PR fixes a real inconsistency that caused broken links and confusion. The solution is clean, well-documented, and aligns with the project's long-standing conventions. Just verify the CI failure and test the dry-run mode before merging.


Review performed by Claude Code following CLAUDE.md guidelines

@ihabadham ihabadham force-pushed the ihabadham/fix/remove-v-prefix-from-git-tags branch from 4dcffed to d2e2bc7 Compare December 30, 2025 17:51
@claude
Copy link

claude bot commented Dec 30, 2025

Code Review - PR #2268

Summary

This PR successfully restores the historical git tag naming convention by removing the v prefix, fixing an inconsistency introduced in PR #1856. The changes are well-documented and address a real issue that caused problems like broken CHANGELOG links.


✅ Strengths

1. Well-Researched and Justified

  • Excellent historical analysis showing 222 tags without v prefix vs 30 with v prefix
  • Clear explanation of how the issue was introduced (release-it auto-detection → manual implementation)
  • Good documentation of the temporary exception (v16.2.0.beta.0 through v16.2.0.rc.0)

2. Comprehensive Documentation Updates

  • Updated all relevant documentation files
  • Added helpful notes about historical tags with v prefix
  • Provides clear guidance for future releases

3. Minimal, Focused Changes

  • Only 3 files changed with surgical precision
  • Clear comments explaining the convention in release.rake:249-250

⚠️ Issues Found

1. CRITICAL: Outdated Comment in update_changelog.rake

File: react_on_rails/rakelib/update_changelog.rake:49

Current state:

# Git tags use 'v' prefix (e.g., v16.2.0), but CHANGELOG uses versions without it

Issue: This comment is now incorrect after your changes. Git tags no longer use the v prefix as the standard convention.

Recommendation:

# Git tags do NOT use 'v' prefix (e.g., 16.2.0), matching CHANGELOG convention
# Note: Historical tags v16.2.0.beta.0 through v16.2.0.rc.0 have 'v' prefix - strip it

Why this matters: The delete_prefix("v") call on line 51 is actually a defensive measure for the historical anomaly tags, not the standard behavior. Developers reading this code need accurate information.


2. MINOR: Inconsistent Documentation Phrasing

In .claude/commands/update-changelog.md, you mention the historical tags in multiple places with slightly different wording:

  • Line 113: "due to a temporary convention change"
  • Line 217: "due to a temporary convention change"
  • Line 128: (no explanation given)
  • Line 242: (no explanation given)

Recommendation: For consistency, use the same clear explanation everywhere:

Note: Historical tags v16.2.0.beta.0 through v16.2.0.rc.0 have a `v` prefix due to 
a temporary convention change in PR #1856 (now reverted).

This provides context and a reference point for anyone investigating.


📋 Test Coverage Assessment

Missing Test Coverage

There are no automated tests for the release script. While this is a pre-existing issue (not your responsibility), it means:

  • ✅ Manual testing is critical - your test plan mentions dry run: rake release[16.3.0,true]
  • ⚠️ Future changes to tag naming could break silently
  • 💡 Consider adding a test that verifies tag format (no v prefix) in the future

Recommendation for this PR: Before merging, please:

  1. Run the dry run: bundle exec rake release[16.3.0,true]
  2. Verify the output shows tag creation without v prefix
  3. Check that the tag validation logic (git rev-parse #{tag_name} on line 252) works correctly

🔒 Security Considerations

No security concerns. This is a pure refactoring of tag naming convention.


⚡ Performance Considerations

No performance impact. The tag creation logic remains unchanged, only the tag name format differs.


🎯 Additional Recommendations

1. Follow-up Task: Cleanup Historical Tags (Optional, Post-Merge)

Consider documenting whether the 30 historical tags with v prefix should be:

  • Left as-is (preferred - maintains git history integrity)
  • Deleted and recreated without v (risky - could break existing references)

My recommendation: Leave them as-is and document them as a historical anomaly (which you've already done well).

2. Update CLAUDE.md Guidelines (Optional Enhancement)

Consider adding a note to CLAUDE.md or docs/contributor-info/releasing.md about the importance of following the established tag convention to prevent future drift.


🎬 Final Verdict

Status: Approve with Required Changes

Required before merge:

  1. ✅ Fix the comment in react_on_rails/rakelib/update_changelog.rake:49
  2. ✅ Run manual dry-run test: bundle exec rake release[16.3.0,true]
  3. ✅ Verify output shows tag name without v prefix

Optional improvements:

  • Consider standardizing the historical tag explanation across all documentation

💯 Overall Assessment

This is a high-quality PR that:

  • Fixes a real inconsistency causing user-facing issues
  • Provides excellent historical context and justification
  • Updates documentation comprehensively
  • Makes minimal, surgical code changes

Great work on identifying and fixing this issue! The one critical change needed is updating the outdated comment in update_changelog.rake.


🤖 Review generated by Claude Code

PR #1856 unintentionally introduced a `v` prefix to git tags when it
replaced `release-it` with manual git commands. The `release-it` tool
auto-detected the existing tag format (no `v` prefix), but the manual
implementation explicitly added `v#{version}`.

This broke 9+ years of consistent tagging convention (versions 2.0.0
through 16.1.2 all used no `v` prefix) and caused issues like broken
CHANGELOG links (fixed in PR #2263).

Changes:
- Remove `v` prefix from tag creation in release.rake
- Update releasing.md to document the correct convention
- Update update-changelog.md to reflect no `v` prefix for future tags
- Add notes about historical v16.2.0.beta.x tags that have `v` prefix

Fixes #2267

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@ihabadham ihabadham force-pushed the ihabadham/fix/remove-v-prefix-from-git-tags branch from d2e2bc7 to 8c27aed Compare December 30, 2025 18:04
@claude
Copy link

claude bot commented Dec 30, 2025

Code Review: Fix git tag v prefix

Summary

This PR correctly restores the historical tag naming convention by removing the v prefix from git tags. The change is well-motivated, properly documented, and addresses a real consistency issue introduced in PR #1856.

✅ Strengths

  1. Well-researched problem: The PR description clearly documents:

  2. Comprehensive documentation updates: All references updated consistently:

    • rakelib/release.rake (the actual fix)
    • docs/contributor-info/releasing.md
    • .claude/commands/update-changelog.md
    • react_on_rails/rakelib/update_changelog.rake
  3. Clear inline comments: The code change at rakelib/release.rake:249-251 includes a helpful comment explaining the rationale.

  4. Proper historical notes: Documentation acknowledges the v16.2.0.beta.0 through v16.2.0.rc.0 tags with v prefix were temporary.

🔍 Code Quality Review

rakelib/release.rake:246-257

# Create git tag (skip if it already exists)
# Note: Tags use version without 'v' prefix (e.g., '16.2.0' not 'v16.2.0')
# to match the historical convention used from version 2.0.0 through 16.1.2
tag_name = actual_gem_version
tag_exists = system("cd #{monorepo_root} && git rev-parse #{tag_name} >/dev/null 2>&1")
if tag_exists
  puts "Git tag #{tag_name} already exists, skipping tag creation"
else
  sh_in_dir(monorepo_root, "git tag #{tag_name}")
end

Clean implementation: Simply changes v#{actual_gem_version}actual_gem_version
Good comment: Explains the rationale and provides an example
Idempotent: Tag existence check prevents duplicate tag errors

📝 Documentation Review

The documentation updates are thorough and consistent:

  1. releasing.md: Updated example from vX.Y.ZX.Y.Z with explanatory note

  2. update-changelog.md:

    • Removed incorrect statement that tags use v prefix
    • Added historical notes about the temporary v16.2.0.beta.x tags
    • Instructs to strip v prefix from historical tags when adding to changelog
  3. update_changelog.rake: Updated comment to acknowledge historical tags may have v prefix

⚠️ Testing Considerations

CRITICAL: This PR modifies the release script, which is infrastructure code that MUST be tested manually before the next release.

Per CLAUDE.md testing-build-scripts.md, before the next release:

# Test with dry run
rake release[16.3.0,true]

# Verify it would create tag without 'v' prefix
# Check the output shows: "git tag 16.3.0" NOT "git tag v16.3.0"

Recommendation: Add this test plan to the PR description or create a reminder issue to test before 16.3.0 release.

🐛 Potential Issues

Minor: CHANGELOG link for 16.2.0.rc.0 still uses v prefix

I noticed in the current CHANGELOG.md:

[16.2.0.rc.0]: https://github.com/shakacode/react_on_rails/compare/16.1.1...v16.2.0.rc.0

This link uses v16.2.0.rc.0 as the ending tag. While the tag DOES exist with that name (it was created during the period when the release script had the v prefix), this is inconsistent with the rest of the CHANGELOG.

Two options:

  1. Leave as-is: The link is technically correct since the tag v16.2.0.rc.0 exists in the repo
  2. Fix for consistency: Change to 16.2.0.rc.0 (GitHub accepts both formats)

I'd lean toward option 1 (leave as-is) since:

  • The link works correctly
  • It accurately reflects the historical tag name
  • Changing it might break existing references

🔒 Security Considerations

✅ No security concerns - this is a versioning/tagging change only.

📊 Performance Considerations

✅ No performance impact - this is a one-time operation during releases.

✅ Best Practices Adherence

Per CLAUDE.md guidelines:

  • Documentation updated: All relevant docs updated consistently
  • Comments explain why: Inline comment at rakelib/release.rake:249-250
  • Fixes root cause: Addresses the actual source of the inconsistency
  • No over-engineering: Minimal change, directly fixes the issue
  • ⚠️ Testing: Needs manual testing before next release (see above)

🎯 Final Recommendation

Approve with minor testing recommendation

This is a well-researched, properly documented fix for a real consistency issue. The implementation is clean and minimal.

Before merging:

  • ✅ Code quality: Excellent
  • ✅ Documentation: Comprehensive
  • ⚠️ Testing: Add reminder to test with dry run before next release

Suggested addition to PR description:

## Post-merge Testing Plan

Before the next release (16.3.0), verify the fix with a dry run:
```bash
rake release[16.3.0,true]

Confirm output shows git tag 16.3.0 (no v prefix).


Great work on identifying and fixing this historical inconsistency! 🎉

@claude
Copy link

claude bot commented Dec 31, 2025

Code Review - PR #2268

✅ Overall Assessment

This is an excellent fix that restores 9+ years of consistent git tag convention. The PR is well-researched, thoroughly documented, and correctly implemented.


🎯 Strengths

  1. Comprehensive Historical Analysis

    • PR properly identifies that 222 historical tags (2.0.0 through 16.1.2) used NO v prefix
    • Only 30 recent tags (v16.2.0.beta.0 through v16.2.0.rc.0) incorrectly used the v prefix
    • Root cause traced to PR Modernize release process for monorepo #1856 when release-it was replaced with manual git commands
  2. Complete Documentation Updates

    • Updated all relevant files: release.rake, releasing.md, update-changelog.md
    • Added helpful notes about historical v16.2.0.beta.x tags being temporary anomaly
    • Maintained consistency with CHANGELOG.md comparison links (which never used v prefix)
  3. Minimal Risk

    • No GitHub Actions workflows trigger on tags (verified via refs/tags search)
    • CHANGELOG comparison links already expect no v prefix
    • CI is passing (all 28 checks green)
  4. Clear Intent

    • Inline comment in release.rake explains WHY no prefix is used
    • Documentation explicitly notes this matches "historical convention"

🔍 Code Quality

rakelib/release.rake (lines 248-255)

# Create git tag (skip if it already exists)
# Note: Tags use version without 'v' prefix (e.g., '16.2.0' not 'v16.2.0')
# to match the historical convention used from version 2.0.0 through 16.1.2
tag_name = actual_gem_version  # ✅ Changed from "v#{actual_gem_version}"

Perfect - Clear comment explains the reasoning
Consistent - Uses the same variable throughout tag creation/checking
Safe - Existing tag_exists check prevents duplicates

Documentation Updates

  • .claude/commands/update-changelog.md: ✅ Updated 4 instances with historical context
  • docs/contributor-info/releasing.md: ✅ Updated tag deletion example
  • react_on_rails/rakelib/update_changelog.rake: ✅ Updated comment to reflect new reality

📋 Verification Checklist

✅ All CI checks passing
✅ No GitHub Actions workflows affected
✅ CHANGELOG comparison links remain unchanged (already using no v prefix)
✅ Documentation consistently updated
✅ Historical context properly documented
✅ Ruby linting passing (RuboCop clean)
✅ Markdown linting passing


🚀 Recommendations

Before Merging:

  1. Already Done - All necessary files updated
  2. Already Done - CI passing
  3. ⚠️ Consider - After the next release (16.3.0), verify the tag was created correctly:
    git tag -l | grep 16.3.0
    # Should show: 16.3.0 (NOT v16.3.0)

Post-Merge:
No action needed. The next release will automatically use the correct tag format.


💡 Minor Suggestions (Optional)

  1. Future Enhancement: Consider adding a test to the release script that validates tag format matches historical convention (e.g., fails if v prefix is accidentally reintroduced)

  2. Documentation Breadcrumbs: The notes about historical v16.2.0.beta.x tags are helpful, but consider adding a one-line note at the TOP of releasing.md for visibility:

    > **Note:** Git tags use version format WITHOUT 'v' prefix (e.g., `16.2.0` not `v16.2.0`). See #2267 for historical context.

✅ Verdict

APPROVED - This PR should be merged. It:

Great work tracking down the root cause and providing such thorough historical analysis! 🎉


Testing Note per CLAUDE.md: This change cannot be fully tested until the next actual release, which is acceptable since:

  • The fix is a simple variable assignment change
  • The code path is well-exercised (every release)
  • The risk is minimal (worst case: manually fix tag if wrong format)
  • CI validates the code compiles and lints correctly

@claude
Copy link

claude bot commented Dec 31, 2025

Code Review - PR #2268

✅ Overall Assessment

This is a well-executed fix that resolves a genuine inconsistency issue. The PR correctly restores the historical git tag convention (no v prefix) that was in use for 9+ years and was unintentionally changed in PR #1856.

All CI checks have passed ✅


🎯 Strengths

  1. Well-researched and documented: The PR clearly explains the historical context, identifies the root cause (PR Modernize release process for monorepo #1856), and provides a complete tag inventory showing the inconsistency.

  2. Minimal, surgical change: The actual code change is just 3 lines in rakelib/release.rake (plus documentation updates), which is exactly what's needed.

  3. Comprehensive documentation updates: All relevant files have been updated:

    • .claude/commands/update-changelog.md - Updated with examples and notes about historical v-prefixed tags
    • docs/contributor-info/releasing.md - Updated tag examples
    • react_on_rails/rakelib/update_changelog.rake - Updated comment to reflect reality
  4. Thoughtful notes about transition period: The documentation correctly notes that historical tags v16.2.0.beta.0 through v16.2.0.rc.0 have the v prefix, which is important for anyone working with those specific versions.

  5. Fixes a real bug: This resolves the CHANGELOG link breakage issue mentioned in PR Fix version link formatting in CHANGELOG.md #2263.


📝 Code Quality

rakelib/release.rake:246-251

# Note: Tags use version without 'v' prefix (e.g., '16.2.0' not 'v16.2.0')
# to match the historical convention used from version 2.0.0 through 16.1.2
tag_name = actual_gem_version

✅ Clear, explanatory comment
✅ Simple variable assignment
✅ No breaking changes to the API

Documentation Changes

  • All documentation updates are accurate and helpful
  • The notes about historical v-prefixed beta/RC tags prevent future confusion
  • Examples use correct tag format

🔍 Considerations

1. Testing the Release Script ⚠️

While the change is simple, I recommend testing the release script in dry-run mode before the next actual release:

rake release[16.3.0,true]

This will verify the tag creation works correctly without actually pushing anything.

2. Historical Tag Cleanup (Optional, not blocking)

You mentioned 30 tags with v prefix vs 222 without. For complete consistency, you could consider retagging the 30 beta/RC versions without the v prefix, but this has tradeoffs:

Pros:

  • Complete consistency across all tags
  • No special-case documentation needed

Cons:

  • Requires force-pushing tags (disruptive to anyone who has checked out those tags)
  • The beta/RC releases are already in production, changing tags could break existing references
  • Not worth the disruption for beta versions

Recommendation: Keep the historical v-prefixed tags as-is (your current approach), and document them as a historical anomaly. Future releases will use the correct format.

3. Changelog Link Format

The PR correctly maintains the changelog link format without v prefix. The changelog links have always been:

[16.2.0.beta.1]: https://github.com/shakacode/react_on_rails/compare/16.1.2...16.2.0.beta.1

Even though git tags temporarily had v prefix, the compare URLs work either way, so this was not broken. Good attention to detail in maintaining consistency.


🔒 Security & Performance

  • ✅ No security implications
  • ✅ No performance impact
  • ✅ No breaking changes for users

📊 Test Coverage

  • ✅ All CI checks passing
  • ✅ No new code that requires tests (documentation + simple variable change)
  • ⚠️ Manual testing recommended: Run rake release[X.Y.Z,true] in dry-run mode before next release

✨ Recommendations

  1. Before merging: Consider adding a test case or verification in the release script that validates the tag format matches the expected pattern (no v prefix). Something like:
# Sanity check: ensure tag doesn't start with 'v'
if tag_name.start_with?('v')
  raise "Tag name should not start with 'v' prefix. Got: #{tag_name}"
end

This would prevent accidental regression in the future.

  1. After merging: Update the contributor guide to explicitly mention that tags should NOT have v prefix, so future contributors don't accidentally reintroduce it.

  2. First release after merge: Pay extra attention to verify the tag is created correctly without v prefix.


🎉 Conclusion

Recommend: APPROVE ✅

This is a solid fix that:

  • Restores historical convention
  • Resolves a real bug
  • Is well-documented
  • Has minimal risk
  • Passes all CI checks

The only minor suggestion is adding a validation check to prevent future regression, but this is not blocking for merge.

Great work on the historical research and thorough documentation! 🙌

@justin808
Copy link
Member

Given the 16.2.0 release is our unified React on Rails Pro release, I think it’s worth considering if we switch to the more standard v version number tags. That is more descriptive and used by our other flagship product, ShakaPacker. I think if we’re going to make the switch, now is a reasonable time to make the switch. The more standard we can be, the easier it is for AI to understand our project. I don’t see any big issue in making the switch to including the v and being consistent going forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inconsistent git tag naming convention: 'v' prefix added in new release script

3 participants