[WIP] Automatic Version Numbering

[jkmassel]

No description provided.

[wpmobilebot]

🚫	Missing space after #.

[wpmobilebot]

🚫	Favor unless over if for negative conditions.

[wpmobilebot]

🚫	Align .compact with .map on line 55.

[wpmobilebot]

🚫	Align .filter with .map on line 55.

[wpmobilebot]

🚫	Align .filter with .map on line 55.

[wpmobilebot]

🚫	Align .sort with .map on line 55.

[wpmobilebot]

🚫	Align .reverse with .map on line 55.

[wpmobilebot]

🚫	Align .first with .map on line 55.

[AliSoftware]

Haven't taken the time to read that code yet (it's still in Draft after all, and there's no PR description yet to explain the idea behind 😛 ) but could this go in the direction of what I had in mind in #203? Or is it about something completely different?

[AliSoftware]

@spencertransier Sorry for taking so long to reply to your Slack request and finally check this PR 😅

I think the Version model could be simplified a lot, and that we should separate responsibilities between objects — in particular I feel like VersionHelper is doing a lot.

[Spoiler: Sorry for the long wall of text ahead; but since you also asked for high-level architecture feedback, I figured you'd be interested in all my rationale and ideas for the bigger picture]

My personal take on how we should probably architecture this would be:

1. Have a simple "bag of properties" model, which would basically be a class (or even just a Struct.new) with only 4 properties :major, :minor, :patch and :prerelease. We could even consider directly using the built-in Gem::Version class for that
2. Have a (set of sub-)class(es) dedicated to parsing and formatting Version objects from and to String representations, based on the formatting convention used by the project
   • For example, we'd have an abstract class VersionFormatter that would expect methods like def parse(string:) taking a String and returning a Version model, a def read_from_file and write_to_file, and def to_build_number(version:) and def to_version_name(version:), taking a Version model and returning a String (typically to be used as the versionCode/VERSION_SHORT and the versionName/VERSION_LONG for iOS/Android)
   • Then we'd provide subclasses of that abstract class for the various formats/conventions we want to support.
   • For example class AndroidVersionFormatter < VersionFormatter would know how to parse and format versions using the x.y.z-rc-n convention, load_from_file would read the version from a .properties file, and to_build_number would generate a string like like 1xxyyzznn
   • While class IOSVersionFormatter would maybe use x.y.z.n format instead, load_from_file would use the Xcodeproj::Config class to read it from .xcconfig file, etc
   • And we could even imagine a class DateVersionFormatter for DayOne-style strings or any other subclasses as needed if we need to support more formats
3. Have a class that we could name VersionBumper, dedicated to know how to bump versions given a "bumping rule" used by the project. For example vb = VersionBumper.new(maxMinor: 12) would be suitable for date-based version format conventions while vb = VersionBumper.new(maxMinor: 9) would be used for WP/WC which bump the major version by one after they reached minor version 9. And if needed, this class could also accept other constraints, like minPatch: 0 parameter to allow to start the next minor version at 0 or at 1, etc

---

With that in place, each client project would:

• Configure e.g. vb = VersionBumper.new(maxMinor: 9), as well as e.g. vf = AndroidVersionFormatter.new(config_file: 'versions.properties', version_code_prefix: 1), at the top of their Fastfile, to declare their overall bumping convention and formatter to be used/shared by all the lanes of that project
• Then when they need to read the current version, they'd use v = vf.load_from_file to get a Version model instance, then vf.to_build_number(v) to format it as a Build Number or vf.to_version_name(v) to format it as a x.y.z-rc-n versionName
• If we need to bump the version in the version file, v = vf.load_from_file; v = vb.bump_minor(v); vf.write_to_file(v) would do the trick
• etc.

---

IMHO separating those concerns and responsibilities this way, with one "dump" model and two separate classes for formatting vs bumping, has many advantages compared to trying to do everything in a big VersionHelper or by trying to implement formatting within the Version model like has been done here.

The main benefit are that:

• This makes it very flexible, allowing to pick and combine the right helpers (formatter + bumper classes) for the project
  • as opposed to having the model object implement many different methods like def android_version_name and def ios_version_number, then allowing an Android client to call ios_version_number even if it wouldn't make sense in the context of an Android project, etc.
• The "rules and conventions" used by the project can be declared in one place, at the top of the Fastfile once and for all.
  • Then the lane would just have to use formatter.load_from_file or bumper.bump_minor when then need to perform those operations, without having to repeat at each call site which convention to use
  • As opposed to having to repeat the path to the version file at every call site, or to make sure that all lanes call that call android_version_code provide the same prefix: value consistently, …
• This also means that in the implementation of your lanes, you won't need to even know or account for which convention is being used by the project.
  • The calls in your lane will look like v = vf.load_from_file, vb.bump_minor(v)… regardless of the convention used by the project
  • Which means you can easily copy/paste similar lanes from one project to another (e.g. from DOiOS to DOAndroid), even if those projects use a different version format and bumping convention. Because the convention used will be declared at the top level, not in each lane's implementation
  • That also means that if we want to change the version convention at some point (e.g. go from x.y.z-rc-n format to x.y.z.n format on a given project), we'd just have to change the VersionFormatter subclass used at the top of the Fastfile. Just one line to change—instead of having to go thru all the lanes and have to think and check what we'd need to update

---

Anyway, this is my personal take and ideas about the architecture that would make things the most flexible for us, but it's also just my personal opinion after all, so feel free to challenge it or play around with the ideas.

[AliSoftware]

delete_prefix already checks if the string starts with said prefix (and is a no-op if it doesn't)

Suggested change

	string = string.delete_prefix('v') if string.start_with?('v')
	string = string.delete_prefix('v')

[AliSoftware]

The doc says that the method is supposed to handle format like x.y.zrc1 too, but this logic won't cover for those cases. To be honest, I'm not convinced that we should support those x.y.zrc1 format in the first place anyway, so maybe just update the method YARD doc comment above instead?

I think the formats we need to support are:

• x.y
• x.y.z
• x.y.z.n
• x.y.z-rcn
• x.y.z-rc-n
  But probably not much more.

[AliSoftware]

Most of the functionality in this model can be implemented by re-using the existing classes available in ruby to manage versions, like Gem::Version.

For example:

def initialize(major:, minor:, patch: nil, rc_number: nil)
  version_string = [major, minor, patch, rc_number].compact.join('.')
  @parts = Gem::Version.new(version_string).canonical_segments
end

def self.create(string)
  # Dashes are considered to mean 'pre' so '1.2.3-rc-4' would give segments [1,2,3,'pre','rc','pre',4]
  @parts = Gem::Version.new(version_string).canonical_segments.reject { |part| part == 'pre' || part == 'rc' }
end

# Return version in format `x.y.z.n`
def to_s
  @parts.join('.')
end

# Return version informat `x.y-rc-n`, or `x.y.z-rc-n` if `z>0`
def android_version_name
  xyz = @parts[2].zero? ? @parts[0..1] : @parts[0..2] # Do not include patch if zero
  rc_suffix = parts[3].nil? ? '' : "-rc-#{parts[3]}"
  xyz.join('.') + rc_suffix
end

We could also consider using Gem::Version's build-in comparison methods to compare two versions

[AliSoftware]

This works for client repos that use that convention that minor versions loop to the next major after 9. But this won't work for e.g. client repos that use date-based versions (like DayOne) where we'd probably loop after 12 instead, or if one day we have a library or tool repo for which we want to use those release-toolkit actions but for which we don't have that constraint of limiting the minor version to <10 (e.g. release-toolkit or dangermattic or similar projects follow SemVer.org semantics so their minor version can be arbitrary high as long as there's no breaking change happening.