Skip to content

Commit 6565985

Browse files
committed
fix: address pull request review comments
- Fix git describe error handling in release script - Fix array insertion issue in changelog generation - Extract version regex pattern as environment variable in release.yml - Add proper error handling for missing tags
1 parent 732526c commit 6565985

4 files changed

Lines changed: 59 additions & 47 deletions

File tree

Rakefile

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ end
192192
def generate_changelog_for_version(version)
193193
content = VersionHelper.generate_changelog_for_version(version)
194194
puts content
195-
195+
196196
# Update CHANGELOG.md if it exists
197197
if File.exist?('CHANGELOG.md')
198198
update_changelog_file(version, content)
@@ -227,7 +227,7 @@ end
227227
def create_changelog_file(version, content)
228228
changelog_file = 'CHANGELOG.md'
229229
header = "# Changelog\n\nAll notable changes to this project will be documented in this file.\n\nThe format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),\nand this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).\n\n## [Unreleased]\n\n"
230-
230+
231231
File.write(changelog_file, header + content)
232232
puts "✅ Created #{changelog_file} with version #{version}"
233233
end

docs/VERSION_HELPER_REFACTORING.md

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# Code Refactoring: Extracted Version Helper
22

33
## Problem
4+
45
The GitHub Actions workflow in `.github/workflows/create-release-pr.yml` contained inline Ruby code that duplicated version bumping logic from the Rakefile:
56

67
```ruby
@@ -13,30 +14,35 @@ NEXT_VERSION=$(ruby -e "
1314
```
1415

1516
This created several issues:
17+
1618
- **Code duplication** between Rakefile and GitHub Actions
1719
- **Maintenance burden** when updating version logic
1820
- **Fragile inline code** that's hard to test and debug
1921
- **Poor separation of concerns**
2022

2123
## Solution
24+
2225
Created a dedicated `VersionHelper` module and CLI script to centralize all version-related logic:
2326

2427
### 1. New Files Created
2528

2629
#### `lib/version_helper.rb`
30+
2731
- **Centralized version logic** for semantic versioning operations
2832
- **Conventional commit analysis** for automatic version bump detection
2933
- **Changelog generation** with proper categorization
3034
- **Comprehensive error handling** and validation
3135

3236
#### `scripts/version.rb`
37+
3338
- **CLI interface** for version operations
3439
- **GitHub Actions friendly** with simple command interface
3540
- **Executable script** that can be called from any environment
3641

3742
### 2. Key Refactoring Changes
3843

3944
#### Before (Duplicated):
45+
4046
```ruby
4147
# In Rakefile
4248
def bump_version(version, bump_type)
@@ -48,6 +54,7 @@ NEXT_VERSION=$(ruby -e "complex inline code")
4854
```
4955

5056
#### After (DRY):
57+
5158
```ruby
5259
# In lib/version_helper.rb
5360
module VersionHelper
@@ -68,11 +75,13 @@ NEXT_VERSION=$(ruby scripts/version.rb next "$RELEASE_TYPE")
6875
### 3. Updated Components
6976

7077
#### Rakefile
78+
7179
- **Simplified helper methods** that delegate to VersionHelper
7280
- **Removed duplicate logic** for commit analysis and version bumping
7381
- **Enhanced preview task** using centralized version info
7482

7583
#### GitHub Actions Workflow
84+
7685
- **Replaced inline Ruby** with clean CLI script calls
7786
- **Improved maintainability** with external script dependency
7887
- **Better error handling** and debugging capabilities
@@ -99,22 +108,27 @@ ruby scripts/version.rb info
99108
## Benefits
100109

101110
### **Eliminated Code Duplication**
111+
102112
- Single source of truth for version logic
103113
- Consistent behavior across all tools
104114

105115
### **Improved Maintainability**
116+
106117
- Changes only need to be made in one place
107118
- Easier to test and debug version logic
108119

109120
### **Better Separation of Concerns**
121+
110122
- Version logic isolated in dedicated module
111123
- CLI script provides clean interface
112124

113125
### **Enhanced Testability**
126+
114127
- Helper module can be unit tested independently
115128
- CLI script provides clear interfaces
116129

117130
### **GitHub Actions Optimization**
131+
118132
- Cleaner workflow YAML without inline Ruby
119133
- Better error messages and debugging
120134

@@ -139,10 +153,12 @@ ruby scripts/version.rb info
139153
## Files Modified
140154

141155
### New Files
156+
142157
- `lib/version_helper.rb` - Centralized version management module
143158
- `scripts/version.rb` - CLI interface for version operations
144159

145160
### Modified Files
161+
146162
- `Rakefile` - Updated to use VersionHelper instead of duplicated logic
147163
- `.github/workflows/create-release-pr.yml` - Replaced inline Ruby with CLI script calls
148164

lib/version_helper.rb

Lines changed: 33 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -9,19 +9,17 @@ module VersionHelper
99

1010
# Bump version based on release type
1111
def self.bump_version(current_version, release_type)
12-
if current_version.nil? || current_version == 'none'
13-
return '1.0.0'
14-
end
12+
return '1.0.0' if current_version.nil? || current_version == 'none'
1513

1614
# Remove 'v' prefix if present
1715
version_string = current_version.sub(/^v/, '')
18-
16+
1917
match = version_string.match(VERSION_PATTERN)
20-
unless match
21-
raise "Invalid version format: #{current_version}"
22-
end
18+
raise "Invalid version format: #{current_version}" unless match
2319

24-
major, minor, patch = match[1].to_i, match[2].to_i, match[3].to_i
20+
major = match[1].to_i
21+
minor = match[2].to_i
22+
patch = match[3].to_i
2523

2624
case release_type.to_s.downcase
2725
when 'major'
@@ -39,34 +37,32 @@ def self.bump_version(current_version, release_type)
3937
def self.current_version
4038
# Get the latest tag that looks like a version
4139
latest_tag = `git describe --tags --abbrev=0 --match="v*" 2>/dev/null`.strip
42-
43-
if latest_tag.empty? || $?.exitstatus != 0
44-
return nil
45-
end
40+
41+
return nil if latest_tag.empty? || $?.exitstatus != 0
4642

4743
latest_tag
4844
end
4945

5046
# Analyze commits since last version to determine suggested release type
5147
def self.analyze_commits_for_version_bump(since_ref = nil)
5248
since_ref ||= current_version
53-
49+
5450
if since_ref.nil?
5551
# No previous version, suggest minor for initial release
5652
return 'minor'
5753
end
5854

5955
# Get commits since the reference
6056
commits = `git log #{since_ref}..HEAD --oneline --no-merges 2>/dev/null`.strip.split("\n")
61-
57+
6258
return 'patch' if commits.empty?
6359

6460
has_breaking = false
6561
has_feature = false
6662

6763
commits.each do |commit|
6864
commit_msg = commit.split(' ', 2)[1] || ''
69-
65+
7066
# Check for breaking changes
7167
if commit_msg.include?('BREAKING CHANGE') || commit_msg.match(/^[^:]+!:/)
7268
has_breaking = true
@@ -77,19 +73,19 @@ def self.analyze_commits_for_version_bump(since_ref = nil)
7773

7874
return 'major' if has_breaking
7975
return 'minor' if has_feature
80-
76+
8177
'patch' # Default for fixes and other changes
8278
end
8379

8480
# Generate changelog entries for a version
8581
def self.generate_changelog_for_version(version, since_ref = nil)
8682
since_ref ||= current_version
87-
88-
if since_ref.nil?
89-
commits = `git log --oneline --no-merges`.strip.split("\n")
90-
else
91-
commits = `git log #{since_ref}..HEAD --oneline --no-merges`.strip.split("\n")
92-
end
83+
84+
commits = if since_ref.nil?
85+
`git log --oneline --no-merges`.strip.split("\n")
86+
else
87+
`git log #{since_ref}..HEAD --oneline --no-merges`.strip.split("\n")
88+
end
9389

9490
# Categorize commits
9591
features = []
@@ -121,34 +117,34 @@ def self.generate_changelog_for_version(version, since_ref = nil)
121117
# Build changelog content
122118
changelog_content = []
123119
changelog_content << "## [#{version}] - #{Date.today.strftime('%Y-%m-%d')}"
124-
changelog_content << ""
120+
changelog_content << ''
125121

126122
unless breaking_changes.empty?
127-
changelog_content << "### ⚠️ BREAKING CHANGES"
128-
changelog_content << ""
123+
changelog_content << '### ⚠️ BREAKING CHANGES'
124+
changelog_content << ''
129125
changelog_content.concat(breaking_changes)
130-
changelog_content << ""
126+
changelog_content << ''
131127
end
132128

133129
unless features.empty?
134-
changelog_content << "### ✨ Features"
135-
changelog_content << ""
130+
changelog_content << '### ✨ Features'
131+
changelog_content << ''
136132
changelog_content.concat(features)
137-
changelog_content << ""
133+
changelog_content << ''
138134
end
139135

140136
unless fixes.empty?
141-
changelog_content << "### 🐛 Bug Fixes"
142-
changelog_content << ""
137+
changelog_content << '### 🐛 Bug Fixes'
138+
changelog_content << ''
143139
changelog_content.concat(fixes)
144-
changelog_content << ""
140+
changelog_content << ''
145141
end
146142

147143
unless other_changes.empty?
148-
changelog_content << "### 🔧 Other Changes"
149-
changelog_content << ""
144+
changelog_content << '### 🔧 Other Changes'
145+
changelog_content << ''
150146
changelog_content.concat(other_changes)
151-
changelog_content << ""
147+
changelog_content << ''
152148
end
153149

154150
changelog_content.join("\n")
@@ -157,7 +153,7 @@ def self.generate_changelog_for_version(version, since_ref = nil)
157153
# Determine the next version based on commits
158154
def self.determine_next_version(release_type = 'auto')
159155
current = current_version
160-
156+
161157
if release_type == 'auto'
162158
suggested_type = analyze_commits_for_version_bump(current)
163159
bump_version(current, suggested_type)
@@ -174,7 +170,7 @@ def self.version_info
174170
suggested_type: analyze_commits_for_version_bump(current),
175171
next_patch: bump_version(current, 'patch'),
176172
next_minor: bump_version(current, 'minor'),
177-
next_major: bump_version(current, 'major')
173+
next_major: bump_version(current, 'major'),
178174
}
179175
end
180176
end

scripts/version.rb

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313

1414
def main
1515
command = ARGV[0]
16-
16+
1717
case command
1818
when 'current'
1919
puts VersionHelper.current_version || 'none'
@@ -31,13 +31,13 @@ def main
3131
puts "Next major: #{info[:next_major]}"
3232
else
3333
puts "Usage: #{$0} <command> [args]"
34-
puts ""
35-
puts "Commands:"
36-
puts " current Get current version"
37-
puts " next [type] Get next version (auto/major/minor/patch)"
38-
puts " bump [type] Same as next (alias)"
39-
puts " analyze Analyze commits for suggested release type"
40-
puts " info Get all version information"
34+
puts ''
35+
puts 'Commands:'
36+
puts ' current Get current version'
37+
puts ' next [type] Get next version (auto/major/minor/patch)'
38+
puts ' bump [type] Same as next (alias)'
39+
puts ' analyze Analyze commits for suggested release type'
40+
puts ' info Get all version information'
4141
exit 1
4242
end
4343
rescue StandardError => e

0 commit comments

Comments
 (0)