Open Cheddam opened 4 years ago
No objections from me. Version-based makes more sense than any of the other terms IMO - develop
/next
/main
arguably all imply that changes merged into them will be in the “next release” when it will likely be a long while before they’re released.
I have no issue with "master", but if the majority want to change it then I'm on board with version based as well.
Last time I checked the userhelp site relies on master branches existing, so that'll need to be updated at the same time too.
Some of our CI builds will break due to dependencies on 'master' branches rather than version aliases.
We can swap uses of dev-master
for branch aliases now to make this smoother, provided it doesn't add to much maintenance overhead for the CMS squad to do so. It'd make sense to follow up on https://github.com/silverstripe/silverstripe-framework/issues/9560 first though.
There's a small concern that renaming it to 5 might be misinterpreted by some as a significant milestone in 5's development, which would be misleading. However, given that it's still 5.x-dev
and more people interact with composer than directly with git, this is probably okay.
I would probably couple this with some kind of note in the readme explaining the branch status of 3, 3.x, 4, 4.x 5, and trusty ole post-2.4.
Sounds like we're doing this?
I'm a bit out of touch with the latest usage in composer of things like dev-master
, given the recent trend to rename master branches maybe composer handles this gracefully now?
But I would er on the side of not renaming master
, I think using version numbers pushes us into a corner a little and will potentially have a wide ranging impact across consumers and contributors that I'm not really sure is worth the benefit.
I'm not opposed to the renaming, but I think we need to be cautious of unintended consequences and decide if there benefits out-weigh the value
Background
For repositories in the Silverstripe CMS core / supported module ecosystem, 'master' generally isn't the default branch - instead, it represents the 'next major', where Semver-breaking changes are merged. With this in mind, the term 'master' is pretty abstract and a little misleading, particularly given that, as of writing, the next major CMS release isn't yet under active development.
Using named instead of numbered branches also presents some overhead, particularly in the form of Composer branch aliases. Updating these aliases is pure toil, and though it's now a very irregular activity (due to them only being applied to 'master' branches), it seems unnecessary when simply naming the branches after what they represent is an option.
Proposal
Let's rename the
master
branches across thesilverstripe
organisation, starting with core modules, to the major version they actually represent (5
or2
in most cases.)This would have the benefit of obviating the need for Composer branch aliases (which we've already removed from 'next minor' branches, and which we're discussing the utility of for
master
branches.)The easiest and most sensible time to action this change may be when we prepare to kick off active development on CMS 5.
Impact
dev-master
are aware of it and can prepare (by switching these references to5.x-dev
/2.x-dev
.) If we wanted to avoid breaking these references, we could look into reversing the branch aliases, but I think ripping the band-aid off is probably better in the long term.Other considerations
If the version-based branch name approach doesn't appeal, here are some alternative names to consider that still drop the 'master' terminology and potentially improve understanding (bearing in mind that these would not allow us to remove branch aliases):
next
: Commonly used in the JS ecosystem to represent a preview of the 'next' major release.develop
: Used in Git Flow to represent approved but unreleased code.unstable
: Would give clearer guidance on the state of the branch than 'master'.main
: The new default branch name GitHub intends to implement. Based on our release processes, and given that this won't actually be the 'main' branch, I don't think this option makes a lot of sense.