-
Notifications
You must be signed in to change notification settings - Fork 9
Add Protected Branch Merge Strategy #44
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
Open
leonevich
wants to merge
7
commits into
unblu:main
Choose a base branch
from
leonevich:protected-branch-merge-strategy
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
ae32f34
Add Protected Branch Merge Strategy
f01b32f
Add Protected Branch Merge Strategy
4f39f11
Add Protected Branch Merge Strategy
366d905
Add Protected Branch Merge Strategy
94c9e6c
Add Protected Branch Alphanumeric Sort
7d0a9b0
cascade merge only valid protected branches
82888d9
Renamed The Method
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| package enums; | ||
|
|
||
| public enum MergeStrategy { | ||
| CONFIGURATION_FILE, | ||
| PROTECTED_BRANCHES | ||
|
|
||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.xcomes beforerelease/9.x.x, which is not a natural merge path.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 nameddev,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.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 -> ...Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with your analysis, and that's why we ended up with a configuration file to define the merge paths.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This solution is not universal, but in my case, where only release branches are protected, it works great