Skip to content

Conversation

@iangmaia
Copy link
Contributor

@iangmaia iangmaia commented Oct 30, 2025

See AINFRA-1478

Description

This PR enhances the version handling added in the previous iteration to assume the version received by the code freeze lane as the source of truth, ignoring potential mismatches between the ReleasesV2 version and the project's computed version.

Testing

You can make sure the calculated release branch, current version and the next version are correctly shown in the "Continue" prompt, rememberomg to answer N when asked to continue to cancel the code freeze.

To avoid unexpected results, comment out the lines doing ensure_git_status_clean and Fastlane::Helper::GitHelper.checkout_and_pull(DEFAULT_BRANCH) at the beginning of the lane.

You can run the start_code_freeze lane, with and without the version parameters:

  • Run bundle exec fastlane start_code_freeze
  • Run bundle exec fastlane start_code_freeze version:30.6

This will be fully tested in the next release cycle during code freeze.

@iangmaia iangmaia self-assigned this Oct 31, 2025
@iangmaia iangmaia added the category: tooling Anything that involves building & maintaining the project, including scripts, `Fastfile`, etc. label Oct 31, 2025
@iangmaia iangmaia added this to the 23.7 milestone Oct 31, 2025
@iangmaia iangmaia force-pushed the iangmaia/fail-when-code-freeze-version-mismatch branch from b81c8fb to 8e7ef55 Compare October 31, 2025 22:13
@iangmaia iangmaia marked this pull request as ready for review October 31, 2025 22:14
@AliSoftware
Copy link
Contributor

I'm not sure I understand why this is a good idea.

I can imagine edge cases where it would be totally legit for the version provided by ReleasesV2 is not matching the "next version" computed from the version.properties files, including:

  • When a version has been skipped for some reason. e.g. version 12.3 was code-frozen as planned, but some unexpected event made the team decide to cancel that release and not release it after all, so they end up leaving the release/12.3 branch open but never merge it, or even decide to close it as they cancel the 12.3 release in ReleasesV2. Then later they start the code-freeze of 12.4 from ReleasesV2, which would trigger the new error
  • If the team decides to make a special version bump; maybe less relevant for WP/JP/Woo—which iinm all use the "increment major version when minor version reaches 9" logic—but more relevant for products like e.g. DayOne which might decide to bump the major version on a year change or Pocket Casts which might do so when a significant change of the app is about to ship and they want to market it with a big version bump…

All in all I'm thus not convinced that this change of making this an error is the right move. I'd even suggest that ReleasesV2 should always be considered the only source of truth and that we maybe don't even bother checking if the "next version based on version.properties would have matched it or not"?

@iangmaia
Copy link
Contributor Author

iangmaia commented Nov 3, 2025

I'm not sure I understand why this is a good idea.

I can imagine edge cases where it would be totally legit for the version provided by ReleasesV2 is not matching the "next version" computed from the version.properties files, including:

  • When a version has been skipped for some reason. e.g. version 12.3 was code-frozen as planned, but some unexpected event made the team decide to cancel that release and not release it after all, so they end up leaving the release/12.3 branch open but never merge it, or even decide to close it as they cancel the 12.3 release in ReleasesV2. Then later they start the code-freeze of 12.4 from ReleasesV2, which would trigger the new error
  • If the team decides to make a special version bump; maybe less relevant for WP/JP/Woo—which iinm all use the "increment major version when minor version reaches 9" logic—but more relevant for products like e.g. DayOne which might decide to bump the major version on a year change or Pocket Casts which might do so when a significant change of the app is about to ship and they want to market it with a big version bump…

All in all I'm thus not convinced that this change of making this an error is the right move. I'd even suggest that ReleasesV2 should always be considered the only source of truth and that we maybe don't even bother checking if the "next version based on version.properties would have matched it or not"?

This came mainly from a quick discussion we had at the meetup following up to this issue. Of course didn't discuss it very deeply, but I recall us coming to the conclusion that an error would've avoided this case and we should probably switch to an error, as in this set of PRs. This issue admittedly is also an edge case... but there are many others, as just simply retrying code freeze multiple times -- without guardrails, as is this lane probably will always be a point for issues and require manual intervention.

Though I fully get the cases you pointed. I think we're maybe missing something that will take us to some balance between a very strict validation that will potentially block folks and letting the lane to be freely ran and just deal with the relatively frequent consequences. Maybe a proper fix would require changes in the UI, like flag like "Force Rv2 Version". Or just use the Rv2 version directly without the check, as you also mentioned.

I'd even suggest that ReleasesV2 should always be considered the only source of truth and that we maybe don't even bother checking if the "next version based on version.properties would have matched it or not"

This is something I've also been thinking while I was adding the validation. Given the current code, the changes for this to fully work are a bit more subtle. There are a lot of version calculations (beta, next milestone release, etc), utils that use the files, each repo is a bit different.... Maybe receiving the version from Rv2 and setting it in the version.properties file is a good compromise given a lot of code relies on what's in the file 🤔 Wdyt? I could perhaps shift these PRs to that approach, as it sounds safer at least for now.

@AliSoftware
Copy link
Contributor

This came mainly from a quick discussion we had at the meetup following up to p1760445236847599/1760442914.028979-slack-C06CKSPHYA1. Of course didn't discuss it very deeply, but I recall us coming to the conclusion that an error would've avoided this case and we should probably switch to an error, as in this set of PRs. This issue admittedly is also an edge case... but there are many others, as just simply retrying code freeze multiple times -- without guardrails, as is this lane probably will always be a point for issues and require manual intervention.

What about just validating that the ReleasesV2 version is not already set in the version.properties, to detect potential re-run of the code-freeze?
Or, even better, detect other side-effects telling that the code freeze already happened, in particular checking things that would be the cause of the rest of the lane breaking on re-run (e.g. if the release/* branch already exists, etc)? After all, if the goal is to avoid things from breaking in case the code-freeze is retried, that's what we should test for (and error on), instead of testing if the version bump dictated by ReleasesV2 would match the one we'd assume was the next version or not…

This is something I've also been thinking while I was adding the validation. Given the current code, the changes for this to fully work are a bit more subtle. There are a lot of version calculations (beta, next milestone release, etc), utils that use the files, each repo is a bit different.... Maybe receiving the version from Rv2 and setting it in the version.properties file is a good compromise given a lot of code relies on what's in the file 🤔 Wdyt? I could perhaps shift these PRs to that approach, as it sounds safer at least for now.

Yep, that's what I was gonna suggest next. The important part for which ReleasesV2 has to be the one dictating the version is when we do the code freeze, since that's the only moment during which we update the version to a new value and thus where we want ReleasesV2 to be the source of truth for what that new version should be, and thus which version to bump the version.properties file to.

For all other lanes after the code_freeze:

  • It's ok to still rely on the version.properties and the helpers (at least for now), as those lane operate on the release/${RELEASE_VERSION} branch that has already been code-frozen and the version.properties is already expected to have the same version as the name of the release/* branch we're running that code from
  • In the long run we could consider making it an error if the $RELEASE_VERSION provided by ReleasesV2 during those post-code-freeze tasks (e.g. new_beta_release, finalize_release, …) do not match the version set in version.properties, just as an extra guardrail. That being said, I'm not sure that ever happened that the main versionName was inconsistent with ReleasesV2's version and the release/* branch name in the past? So I'm not sure that particular guardrail would be absolutely necessary at that point.

@iangmaia iangmaia force-pushed the iangmaia/fail-when-code-freeze-version-mismatch branch from 8e7ef55 to 42cee9e Compare November 7, 2025 15:19
@iangmaia iangmaia changed the title [Tooling] Fail on Version Mismatch Between ReleasesV2 and Project [Tooling] Use ReleasesV2 version as the source of truth during Code Freeze Nov 7, 2025
@iangmaia iangmaia force-pushed the iangmaia/fail-when-code-freeze-version-mismatch branch from 64c05f2 to 4af8a15 Compare November 7, 2025 21:42
@iangmaia
Copy link
Contributor Author

iangmaia commented Nov 7, 2025

@AliSoftware I've updated this PR (as well as a couple of others with the PR title and description, the rest will follow) following a simpler approach of just taking the version from ReleasesV2. I've tweaked the beta version used throughout the lane and a couple of minor things, but I think it looks better now.

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

Labels

category: tooling Anything that involves building & maintaining the project, including scripts, `Fastfile`, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants