Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ _None_

### New Features

_None_
- Add the possibility to configure in DerivedBuildCodeFormatter a versioning prefix instead of always defaulting to 1. [#656]

### Bug Fixes

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,19 @@ module Versioning
# The `DerivedBuildCodeFormatter` class is a specialized build code formatter for derived build codes.
# It takes in an AppVersion object and derives a build code from it.
class DerivedBuildCodeFormatter
# Initialize the formatter with a configurable prefix.
#
# @param [String] prefix The prefix to use for the build code. Must be a single digit (0-9), or empty string / nil.
#
def initialize(prefix: nil)
prefix ||= ''
validate_prefix!(prefix)
@prefix = prefix.to_s
end

# Calculate the next derived build code.
#
# This method derives a new build code from the given AppVersion object by concatenating the digit 1,
# This method derives a new build code from the given AppVersion object by concatenating the configured prefix,
# the major version, the minor version, the patch version, and the build number.
#
# @param [AppVersion] version The AppVersion object to derive the next build code from.
Expand All @@ -19,15 +29,43 @@ class DerivedBuildCodeFormatter
# @return [String] The formatted build code string.
#
def build_code(build_code = nil, version:)
Copy link
Contributor

@AliSoftware AliSoftware Aug 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not for this PR, but mentioning while I saw it:

Note that this build_code parameter is unused, but was initially still kept here so that the common API for all the *BuildCodeFormatter have the same API (as mentioned in the YARD comment about it), making it easier to switch from one formatter to another without changing the call sites (abstraction layer pattern).

But in practice I just realized that:

  1. We don't have an abstract class for all the *BuildCodeFormatter classes like we have for AbstractVersionFormatter. In fact, the *BuildCodeFormatter classes don't inherit any common parent class, unlike the *VersionFormatter classes
  2. In practice the API for all *BuildCodeFormatter classes is not even consistent (which might actually explain point 1). For example this DerivedBuildCodeFormatter as well as FourPartBuildCodeFormatter indeed both have a signature of def build_code(build_code = nil, version:)… but the SimpleBuildCodeFormatter uses def build_code(version = nil, build_code:) instead…

That makes me think that (1) either one day we should create an abstract class for the *BuildCodeFormatter and unify their API for consistency (2) or we should just abandon the idea of having unified API for all (given the needed input parameters might depend on what the formatter does and needs to compute its value), and remove the unused parameters from the def build_code(…) signatures for clarity… 🙃


Also, in any case, I am surprised that rubocop didn't complain about it and didn't suggest us to rename this parameter with an underscore to indicate that it being unused is intentional (def build_code(_build_code = nil, version:) 🤷

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes me thing that (1) either one day we should create an abstract class for the *BuildCodeFormatter and unify their API for consistency (2) or we should just abandon the idea of having unified API for all

Oh, agreed.
I quickly saw the parameter and imagined there would be a base, abstract class like the other versioning formatters.

I am surprised that rubocop didn't complain about it and didn't suggest us to rename this parameter

True 🤔

format(
# 1 is appended to the beginning of the string in case there needs to be additional platforms or
# extensions that could then use a different digit prefix such as 2, etc.
'1%<major>.2i%<minor>.2i%<patch>.2i%<build_number>.2i',
result = format(
# The prefix is configurable to allow for additional platforms or
# extensions that could use a different digit prefix such as 2, etc.
'%<prefix>s%<major>.2i%<minor>.2i%<patch>.2i%<build_number>.2i',
prefix: @prefix,
major: version.major,
minor: version.minor,
patch: version.patch,
build_number: version.build_number
)

result.gsub(/^0+/, '')
end

private

# Validates that the prefix is a valid single digit (0-9) or empty string.
#
# @param [String] prefix The prefix to validate
#
# @raise [StandardError] If the prefix is invalid
#
def validate_prefix!(prefix)
prefix_str = prefix.to_s

# Allow empty string
return if prefix_str.empty?

# Check if it's longer than 1 character
if prefix_str.length > 1
UI.user_error!("Prefix must be a single digit or empty string, got: '#{prefix_str}' (length: #{prefix_str.length})")
end

# Check if it's a valid integer
return if ('0'..'9').include?(prefix_str)

UI.user_error!("Prefix must be an integer digit (0-9) or empty string, got: '#{prefix_str}'")
end
end
end
Expand Down
118 changes: 110 additions & 8 deletions spec/derived_build_code_formatter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,118 @@

describe Fastlane::Wpmreleasetoolkit::Versioning::DerivedBuildCodeFormatter do
describe 'derives a build code from an AppVersion object' do
it 'derives the build code from version numbers that are single digits' do
version = Fastlane::Models::AppVersion.new(1, 2, 3, 4)
build_code_string = described_class.new.build_code(version: version)
expect(build_code_string.to_s).to eq('101020304')
context 'with default prefix (nil)' do
it 'derives the build code from version numbers that are single digits' do
version = Fastlane::Models::AppVersion.new(1, 2, 3, 4)
build_code_string = described_class.new.build_code(version: version)
expect(build_code_string.to_s).to eq('1020304')
end

it 'derives the build code from version numbers that are two digits' do
version = Fastlane::Models::AppVersion.new(12, 34, 56, 78)
build_code_string = described_class.new.build_code(version: version)
expect(build_code_string.to_s).to eq('12345678')
end
end

context 'with explicit prefix' do
it 'derives the build code with prefix "1"' do
version = Fastlane::Models::AppVersion.new(1, 2, 3, 4)
formatter = described_class.new(prefix: '1')
build_code_string = formatter.build_code(version: version)
expect(build_code_string.to_s).to eq('101020304')
end

it 'derives the build code with prefix "2"' do
version = Fastlane::Models::AppVersion.new(1, 2, 3, 4)
formatter = described_class.new(prefix: '2')
build_code_string = formatter.build_code(version: version)
expect(build_code_string.to_s).to eq('201020304')
end

it 'derives the build code with prefix "0"' do
version = Fastlane::Models::AppVersion.new(12, 34, 56, 78)
formatter = described_class.new(prefix: '0')
build_code_string = formatter.build_code(version: version)
expect(build_code_string.to_s).to eq('12345678')
end
end

context 'with empty prefix' do
it 'derives the build code without prefix and trims leading zeros' do
version = Fastlane::Models::AppVersion.new(1, 2, 3, 4)
formatter = described_class.new(prefix: '')
build_code_string = formatter.build_code(version: version)
expect(build_code_string.to_s).to eq('1020304')
end

it 'derives the build code without prefix for two-digit major version' do
version = Fastlane::Models::AppVersion.new(12, 34, 56, 78)
formatter = described_class.new(prefix: '')
build_code_string = formatter.build_code(version: version)
expect(build_code_string.to_s).to eq('12345678')
end

it 'handles edge case with all zeros after removing leading zeros' do
version = Fastlane::Models::AppVersion.new(0, 0, 0, 1)
formatter = described_class.new(prefix: '')
build_code_string = formatter.build_code(version: version)
expect(build_code_string.to_s).to eq('1')
end
end

context 'with nil prefix' do
it 'treats nil prefix as empty string and derives build code without prefix' do
version = Fastlane::Models::AppVersion.new(1, 2, 3, 4)
formatter = described_class.new(prefix: nil)
build_code_string = formatter.build_code(version: version)
expect(build_code_string.to_s).to eq('1020304')
end

it 'treats nil prefix as empty string for two-digit major version' do
version = Fastlane::Models::AppVersion.new(12, 34, 56, 78)
formatter = described_class.new(prefix: nil)
build_code_string = formatter.build_code(version: version)
expect(build_code_string.to_s).to eq('12345678')
end
end
end

describe 'prefix validation' do
context 'with valid prefixes' do
it 'accepts single digits 0-9' do
(0..9).each do |digit|
expect { described_class.new(prefix: digit.to_s) }.not_to raise_error
end
end

it 'accepts empty string' do
expect { described_class.new(prefix: '') }.not_to raise_error
end

it 'accepts integer inputs' do
expect { described_class.new(prefix: 5) }.not_to raise_error
end
end

it 'derives the build code from version numbers that are two digits' do
version = Fastlane::Models::AppVersion.new(12, 34, 56, 78)
build_code_string = described_class.new.build_code(version: version)
expect(build_code_string.to_s).to eq('112345678')
context 'with invalid prefixes' do
it 'rejects multi-character strings' do
expect { described_class.new(prefix: '12') }.to raise_error(/Prefix must be a single digit or empty string/)
end

it 'rejects non-numeric strings' do
expect { described_class.new(prefix: 'a') }.to raise_error(/Prefix must be an integer digit/)
end

it 'rejects numbers outside 0-9 range' do
expect { described_class.new(prefix: '10') }.to raise_error(/Prefix must be a single digit or empty string/)
expect { described_class.new(prefix: '-1') }.to raise_error(/Prefix must be a single digit or empty string/)
end

it 'rejects symbols and special characters' do
expect { described_class.new(prefix: '@') }.to raise_error(/Prefix must be an integer digit/)
expect { described_class.new(prefix: '#') }.to raise_error(/Prefix must be an integer digit/)
end
Comment on lines +115 to +118
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👌

(this reminded me of this meme 😄 )

end
end
end