Add Protected Branch Merge Strategy #44
Conversation
|
|
||
| private Branch getNextTargetBranchFromProtectedBranches(String gitlabEventUUID, Long projectId, String sourceBranch, String mergeSha) { | ||
| try { | ||
| List<ProtectedBranch> protectedBranches = gitlab.getProtectedBranchesApi().getProtectedBranches(projectId); |
There was a problem hiding this comment.
I'm a bit concerned about relying on the sorting provided by the API call.
Gitlab is returning the branches alphabetically sorted, but natural sorting should be preferred. For instance, release/10.x.x comes before release/9.x.x, which is not a natural merge path.
There was a problem hiding this comment.
I'm still not convinced by this sorting approach.
I don't see this "blind" branch sorting working reliably on repositories without very strict branch naming rules. I actually find it quite dangerous, and I can think of some ways it can mess-up repositories. For instance, hotfix/ branches are typically protected and created on demand. Another problem are branches named dev, develop, main, etc. So unless we find a sorting algorithm that respects git branch conventions, I don't think we should assume we can auto discover merge paths.
If we do find that sorting algorithm, we should be able reduce the API calls here. The list branches call already returns the list of branches with the information if the branch is protected or not. We should try to use that, filter the protected ones, and move on from there.
There was a problem hiding this comment.
Then, I think it's worth removing the block for direct merging of protected branches and leaving only the merge strategy within branch groups. The search within branch groups will always follow the wildcard rules like release/*, hotfix/*, etc., and cascade merges will only occur within the group that contains the source branch.
If we do not use wildcard rules in the project settings and instead specify specific branches such as release/1.2.3, hotfix/some-patch, develop, etc., then cascade merging will not be performed since the project configuration does not include branch grouping rules. On the other hand, we can move the wildcard rule to the configuration and specify that branch groups can be combined based on name templates, similar to how it is configured in the project settings.
There was a problem hiding this comment.
Does it make sense to automate the merging between branches, for example, hotfix/* -> release/*?
If so, it is also possible to set up prioritization between groups and then organize a continuous merge process based on the sequence: fix -> hotfix/5.10.1 -> release/5.10.1 -> (hotfix/5.10.2 if it exists) -> release/5.10.2 -> ...
There was a problem hiding this comment.
Nevertheless, all these attempts to establish a strict sequence for cascading merges do not yield reliable results, since defining the sequence is only possible when the branch name contains a version. Therefore, the cascade merge strategy cannot be based solely on a list of protected branches. To ensure transparent system behavior in all cases, the merge strategy should be built around a list of branch groups formed by branch name patterns, which can be ordered alphanumerically. This list is specified in the configuration as (release/*, hotfix/*, ...). Thus, the responsibility for selecting branches for cascade merging remains with the end user.
There was a problem hiding this comment.
I agree with your analysis, and that's why we ended up with a configuration file to define the merge paths.
There was a problem hiding this comment.
This solution is not universal, but in my case, where only release branches are protected, it works great
| Optional<String> apiTokenApprover; | ||
|
|
||
| @ConfigProperty(name = "protected.branch.merge.strategy", defaultValue = "false") | ||
| boolean protectedBranchMergeStrategy; |
There was a problem hiding this comment.
I would go with an Enum here to leave us the flexibility to add more strategies later if needed. We could start with just CONFIGURATION_FILE (default) and PROTECTED_BRANCHES.
There was a problem hiding this comment.
There is a suggestion to also add a merge strategy with the default branch at the end of the cascade. [PROTECTED_BRANCHES_WITH_DEFAULT]
3 participants
Improvement enabling automatic cascading merges across all protected branches in alphabetical order. Supports protect multiple branches with wildcard rules. Allows abandoning branch model support in the configuration file.