Skip to content

Commit

Permalink
Make sure we lint (conflicts in) merge commits when pushing.
Browse files Browse the repository at this point in the history
Summary:
We had a case where someone pushed a change with a git conflict
markers still in it.  In theory the linter should have caught it, but
it didn't because it wasn't linting the relevant file, which was only
showing up in a merge commit.

I'm not sure exactly how this could have happened: presumably if you
had a conflict in a file, you had some other commit you're pushing
that modified the file, and so we should be linting the file because
of that.  But maybe if you merged in two separate branches, each of
which made a different change to a file?   Probably that's why this
hasn't been a problem until now.

Some internet sleuthing showed that the `--cc` flag, despite its
obscure documentation, does what we want.

Issue: commit a11adb329acf

Test Plan:
I ran
   git log --pretty=format: --name-only --cc --diff-filter=AMR --ignore-submodules 0657fd00d46
and its first output was:
```
services/test-prep/models/plan_checkpoint_list.go
test_prep/models/plan_models.py
```
which picks up the files with the conflict markers.  Running just:
   git log --pretty=format: --name-only --diff-filter=AMR --ignore-submodules 0657fd00d46
omits those two lines and starts with the next block of files.

Reviewers: davidbraley, benkraft

Reviewed By: benkraft

Subscribers: jordano, kevinb

Differential Revision: https://phabricator.khanacademy.org/D64494
  • Loading branch information
csilvers committed Jul 10, 2020
1 parent bf190b8 commit 940de62
Showing 1 changed file with 4 additions and 0 deletions.
4 changes: 4 additions & 0 deletions githook.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,10 @@ def pre_push_hook(_unused_arg_remote_name, _unused_arg_remote_location):
# the name of each added/modified/removed file, separated by NUL.
'--pretty=format:', '--name-only', '--diff-filter=AMR', '-z',

# I'm not sure I totally understand it, but this flag
# lets us see resolved conflicts in merge commits.
'--cc',

# Ignore submodules, because they're likely to have lint rules that
# are different than the repository we're currently linting.
'--ignore-submodules',
Expand Down

0 comments on commit 940de62

Please sign in to comment.