[MER-3309] Add an extra merge commit detection based on commit messages #218
Conversation
Current Aviator status
This PR was merged using Aviator. See the real-time status of this PR on the Aviator webapp. Use the Aviator Chrome Extension to see the status of your PR within GitHub.
|
|
Tested with a repository in this way.
|
| if _, err := repo.Git("fetch", "origin", parent.Name); err != nil { | ||
| return "", errors.WrapIff(err, "failed to fetch %q from origin", parent.Name) | ||
| } | ||
| commitHash, err := repo.RevParse(&git.RevParse{Rev: "FETCH_HEAD"}) |
There was a problem hiding this comment.
nit: can you just make this remotes/origin/{parent.Name} instead of FETCH_HEAD? it's hypothetical but FETCH_HEAD could change out from underneath us in a race condition.
There was a problem hiding this comment.
Depending on the fetch spec, the name of the local remote tracking branch can be different, or it doesn't exist at all. While the race condition does exist, the chance it hits that condition is narrow. vs. this local remote tracking branch not existing can happen permanently (namely GitHub Actions' checkout doesn't have it). So, this is intentionally FETCH_HEAD.
internal/actions/sync_branch.go
Outdated
| @@ -61,6 +61,10 @@ func SyncBranch( | |||
| } | |||
| } else { | |||
| if opts.Fetch { | |||
| fetchHead, err := fetchUpstreamCommits(repo, tx, branch) | |||
There was a problem hiding this comment.
minor: you can move this into the else if below since we're not using it except in that case
There was a problem hiding this comment.
No. This is actually used outside of that case.
Without this upstream fetch, the merge commit won't exist locally. This makes the rebase process in syncBranchRebase unable to rebase on top of the merged commit.
draftcode
force-pushed
the
git-log-merge-commit-detection
branch
from
4da24a5 to
cf853ed
Compare
When a PR gets closed by "Close #123", usually GitHub marks the PR to be closed by certain commit. However, this mechanism is not reliable, and sometimes GitHub doesn't mark it so and just close it. This breaks the merge commit detection since av-cli doesn't have a way to know it from GitHub API. By using git-log, we can figure out whether there's actually a commit that closes the PRs with this comment. This adds that extra detection method. In order to make this work, it moves the timing of git-fetch. This is needed in order for a local repository to have commits in the upstream to check the commit messages.
draftcode
force-pushed
the
git-log-merge-commit-detection
branch
from
cf853ed to
48e1f3e
Compare
When a PR gets closed by "Close #123", usually GitHub marks the PR to be
closed by certain commit. However, this mechanism is not reliable, and
sometimes GitHub doesn't mark it so and just close it. This breaks the
merge commit detection since av-cli doesn't have a way to know it from
GitHub API.
By using git-log, we can figure out whether there's actually a commit
that closes the PRs with this comment. This adds that extra detection
method.
In order to make this work, it moves the timing of git-fetch. This is
needed in order for a local repository to have commits in the upstream
to check the commit messages.