Lint PR merge commit #61
Conversation
jakirkham
force-pushed
the
use_pr_merge_commit
branch
from
5ffbe33
to
ccbaba3
Compare
| ref_merge = repo.refs['pull/{pr}/merge'.format(pr=pr_id)] | ||
| except GitCommandError: | ||
| # Either `merge` doesn't exist because the PR was opened | ||
| # in conflict or it is closed and it can't be the latter. |
There was a problem hiding this comment.
Sometimes the merge ref isn't defined.
First case is when the PR is closed. This case should be handled already by this line (only running on open PRs).
Second case is when the PR has always had merge conflicts since it was opened. If the conflicts were ever resolved in the PR, then this is no longer a concern. However if merge conflicts enter the PR, merge ref is still defined (though incorrectly). This is a case that we do have to handle.
Currently we don't do anything about these here, and instead check on this line for merge conflicts. This seems to work well.
There was a problem hiding this comment.
TL;DR Sometimes there is no merge ref basically must be a merge conflict. This is already caught with PR ( #55 ). So we don't need to do anything here.
jakirkham
force-pushed
the
use_pr_merge_commit
branch
from
ccbaba3
to
048a003
Compare
|
Also can you please take a look at this one when you have a chance, @pelson? |
jakirkham
force-pushed
the
use_pr_merge_commit
branch
3 times, most recently
from
cffecfb
to
b8750a9
Compare
jakirkham
force-pushed
the
use_pr_merge_commit
branch
from
b8750a9
to
42605d8
Compare
|
Merging so as to clear out a piece of PR ( #53 ) and make that one clearer. |
Switches to linting the merge commit of PRs instead of the head commit. This is inline with what many other CIs are doing (like Travis CI and AppVeyor). The main reason to do this is to ensure we are not getting false positives from the linter. For example, imagine our PR has an error due to some part of the recipe that is unchanged, but the base branch contains a correction to this issue. We don't want this linting error as it is a false positive and will go away with a simple merge into the base branch.
It is possible to do this now as we correctly raise merge conflicts as an error thanks to PR ( #55 ). The functionality presented here is a smaller piece of the functionality already in PR ( #53 ) to optionally ignore recipes from the base branch. A test is provided to ensure we are getting the intended behavior by using PR ( #54 ).