Don't stop user from committing merge commits to deploy branches #9
Conversation
templates/pre-commit.protect-master
Outdated
| esac | ||
|
|
||
| # Exit if it's a merge commit | ||
| if [ "$(git rev-list --parents -n 1 HEAD | wc -w)" -gt 2 ]; then |
There was a problem hiding this comment.
Oh, I see, it emits two shas for each parent, so this is right as you have it. Don't mind me.
There was a problem hiding this comment.
Turns out this was wrong anyway, because we need to look at the commit in the middle of being made, so I changed it.
templates/pre-commit.protect-master
Outdated
| if [ "$(git rev-list --parents -n 1 HEAD | wc -w)" -gt 2 ]; then | ||
| exit 0 | ||
| fi | ||
|
|
||
| # Get the known deploy branches from the git config (comma-separated list) | ||
| KNOWN_DEPLOY_BRANCHES=$(git config --get ka.olc.targetBranches) |
There was a problem hiding this comment.
Just a style thing, but I'd move this down after line 46, so that the two cannot-commit checks are together.
| exit 0 | ||
| fi | ||
|
|
||
| # Check if we are in the middle of a rebase |
There was a problem hiding this comment.
I didn't think we wanted to let folks rebase deploy branches?
| # Check if we are in the middle of a merge | ||
| if [ -f ".git/MERGE_HEAD" ]; then | ||
| exit 0 | ||
| fi | ||
|
|
||
| # Check if we are in the middle of a rebase | ||
| if [ -d ".git/rebase-merge" ] || [ -d ".git/rebase-apply" ]; then | ||
| exit 0 | ||
| fi |
There was a problem hiding this comment.
| # Check if we are in the middle of a merge | |
| if [ -f ".git/MERGE_HEAD" ]; then | |
| exit 0 | |
| fi | |
| # Check if we are in the middle of a rebase | |
| if [ -d ".git/rebase-merge" ] || [ -d ".git/rebase-apply" ]; then | |
| exit 0 | |
| fi | |
| # Check if we are in the middle of a merge or rebase. | |
| if [ -f ".git/MERGE_HEAD" -o -d ".git/rebase-merge" -o -d ".git/rebase-apply" ]; then | |
| exit 0 | |
| fi |
(Just a small simplification, it's fine as-is too.)
| case "$CURRENT_BRANCH" in | ||
| master|main|next|deploy*) | ||
| next|deploy*) |
There was a problem hiding this comment.
Actually, I'm not really comfortable with deploy* here. I have non-deploy branches with names like deploy-fastly-fix, for fixes to the deploy-fastly script. It's fine when OLC gets concerned with that and I can override it, but having a pre-commit hook get concerned with it isn't ok. So I'd just take that out, or rename it to just deploy.
Summary:
If I'm bringing in
masterormainand there's a merge conflict, I want to commit without using-n, because I want the rest of the verification things to happen.Issue: FEI-5966
Test plan:
Set up a lot of test branches and different combos of things, and test all cases