Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parse release branch versions using LooseVersion #3726

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jlantz
Copy link
Contributor

@jlantz jlantz commented Jan 2, 2024

Currently release branch version numbers can be only an integer with the format feature/NNN. Using the LooseVersion parser from CumulusCI's utilities to parse the version number allows for greater flexibility in release branch naming.

This branch includes modified release branch parsing logic that uses LooseVersion to parse the version number instead of the current isdigit() check. It then checks if there are only integers in the parsed version number's parts and if so, returns the LooseVersion. This approach is backwards compatible with the previous int-only approach.

For example, if you have the following branches:

  • **feature/260
  • **feature/262

And you wanted to create a new planned patch release of 260 developed in parallel, you could simply create the branch feature/260.1

The branch includes doc updates and tests with full coverage of the changes.

@jlantz jlantz requested a review from a team as a code owner January 2, 2024 17:18
@TedHusted
Copy link

@jlantz We support monthly releases, and for various reasons, we name our branches "release/v$YYYY-$MM", such as "release/v2024-01".

(1) Is there a reason why the matching is not based on a regular expression or some pattern that could be configurable?

Just asking in case you already considered and dismissed the idea.

-- Off-Topic --

In our case, we include the "v" to support an integration with another system that does not support numeric symbols.

If I found a workaround for that issue, we could also go with "2024.01".

I do find it a bit curious that release branches use a feature prefix. We could change that too, but referring to the release branches in this way seems more natural.

For the GitHub tag, we also use release/* -- and here is where we use "release/4.1" for example.

(2) Is avoiding name collisions a driver for using "feature" for the release branches?

We tend to avoid prefixes for our work branches, and just name them for the Jira issues (ABC-123) without a prefix.

(3) I'm not clear on the value of prefixes generally, and I sometimes wonder if they are an example of over-design.

@jlantz
Copy link
Contributor Author

jlantz commented Jan 10, 2024

That's not a bad idea about parsing with a regex, but I'd consider that a separate feature than this one. If you wanted to implement regex without this feature, you'd need a regex that produces an integer. The reason is that CumulusCI has logic to automatically merge to newer release branches so it needs an integer to do a comparison. This PR just changes that logic to parse that integer string with LooseVersion so it can include .'s and sorts the way you'd expect version numbers (or any string of numbers separated by dots) to sort.

Together, this PR and your proposal for regex would mean the regex can produce either a string parsable as an integer or a version number by LooseVersion.

@jlantz
Copy link
Contributor Author

jlantz commented Jan 10, 2024

@TedHusted As for the off-topic portion, I just went through this whole series of discussion and debate with one of my clients over the course of ~2 months. I'll try my best to recall the arguments that led to our decision to stick with feature/ for release feature branches instead of release/...

  • For this client, the main focus of development work is on the next release (N)
  • Minor or hotfix releases for the current branch are rare but sometimes needed
  • Development of future release branches beyond N (N+n) are rare but sometimes needed

In this context, having main, the default branch, represent the next release (N) made the most sense since that represents the main development effort of the team. Whenever a dev wants to start a new branch, they create it from the default branch. By comparison, if N were release/N instead of main, all new branches must be created against whatever branch represents the current N branch. You can create those as child branches like release/N__some-feature, but then release branches are already behaving like feature branches. You could create feature/some-feature from the release/N branch, but consider the scenario where you're working on N and N+1 in parallel. Looking at a feature branch, you'd have to look into the commit history to see which release branch it was created from.

With main + feature/N+1 formatting, all branch names tell you what they're against. If they're a feature in the main line of development on N, they're simple feature branches. If they're for an N+n release, they start with feature/[N+n version]__some-feature making it obvious from the branch name that they're related to the release. This branching hierarchy is also used by CumulusCI for child branch merge downs.

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.

None yet

3 participants