Conversation
ltshb
requested review from
adk-swisstopo,
asteiner-swisstopo,
benschs,
hansmannj,
ltclm,
ltflb-bgdi,
ltkum,
msom,
rebert,
rotwurstesser and
schtibe
Due to auto release note, branch pattern must be followed.
asteiner-swisstopo
left a comment
asteiner-swisstopo
left a comment
There was a problem hiding this comment.
I would be fine with enforcing this.
I am less sure whether the naming convention should not be revised. For example, what I miss are prefixes for refactorings ("refactor") or maintenance ("chore").
There are also conventions like Conventional Branch and in general I have noticed that often git branch types are separated with a slash (/) rather than a dash. So for example hotfix/memory-leak-image-upload instead of hotfix-memory-leak-image-upload.
So maybe worth a small discussion in the Tuesday Tech Talk?
| echo "Branch name is 'main', skipping pattern check" | ||
| exit 0 | ||
| fi | ||
| expected_pattern="^(feature|feat|bugfix|bug|hotfix|hot|data|norn|skip-rn)[/_-].+$" |
There was a problem hiding this comment.
What do norn and skip-rn stand for? These are not mentioned in the doc-guidelines.
There was a problem hiding this comment.
My first guess is no release notes and skip release notes.
ltflb-bgdi
left a comment
ltflb-bgdi
left a comment
There was a problem hiding this comment.
Makes sense. As @asteiner-swisstopo already mentioned, referring to official nomenclatures like conventional branch would make sense. At least we should agree on a prefix for work without a issue. Today, both no-issue and chore are used. Personally I would vote for chore. The way I understand it, this is what currently norn and skip-rn stand for.
|
What @asteiner-swisstopo and @ltflb-bgdi already said 😉 |
|
Maybe instead of enforcing a branch naming, we could should enforce the presence of a label. The branch is anyway deleted once merged and if we enforce a naming, then the PR needs to be closed and reopened. While labels can be updated. |
ltclm
left a comment
ltclm
left a comment
There was a problem hiding this comment.
Thank you! Good idea with the labels.
If we stick to the branch naming convention, I'd suggest keeping it in sync with what's documented here: https://github.com/geoadmin/doc-guidelines/blob/master/GIT_FLOW.md#personal-branches-naming-convention
Note that norn, skip-rn, and hot are currently missing from those guidelines.
|
@hansmannj @ltflb-bgdi @asteiner-swisstopo I updated the PR for branch name convention to label enforcement. I haven't tested it yet. |
The branch name was only used to auto-assign a label for release note categorization, and the branch is deleted once the PR is merged, so its naming is not important in itself. Labels are what actually matter: they categorize release notes and can now also be used to block accidental merges (e.g. via a "DO NOT MERGE" label), so validate labels directly instead of enforcing a branch naming convention.
5 participants
Github branch nomenclature is used to automate PR labels which are then used to categorize Release Notes. Branch name otherwise are not so important and they are automatically deleted once the PR is merged. Therefore any convention enforcement do not really make sense and it would also be quite annoying because once a PR is open, you cannot change the name of the head branch, in that case you need to close the PR and reopen a new one.
However the PR labels are important and should be enforced for proper Release Note Categorization. That's why this PR now implement a supplementary github action that can be used with PR label and unlabel events. I also added a PR merge guards to prevent accidental merge of PR labeled as DO NOT MERGE or WIP.
This is a new reusable workflow which means that it needs to be added to every github repo. See https://github.com/swissgeo/infra-terraform/pull/423 for the workfow setup