unicode-org / jira-github-pr-check

Checks GitHub pull requests for valid and accepted Jira tickets. Used for ICU and CLDR
Other
11 stars 12 forks source link

P1 Auto-approve PRs that are force-pushed #6

Open sffc opened 5 years ago

sffc commented 5 years ago

Context: https://github.community/t5/How-to-use-Git-and-GitHub/Auto-approve-force-pushes-if-PR-was-already-approved/m-p/15029/thread-id/4672

The checker tool should be able to detect when the only change to a PR was a force-push to squash, edit commit message, etc., and it should automatically re-approve the PR.

If not auto-approve, the bot should at least leave a comment saying that the code is the same, so that the human reviewer doesn't have to take that on faith.

@srl295

sffc commented 5 years ago

For the record: a related issue that I filed with GitHub support:

The following URL shows two branches as having identical content, similar to what I would see on the command line if I ran "git diff branch_01..branch_02".

https://github.com/Labutin/CompareBranches/compare/branch_01..branch_02

This is the correct, expected behavior.

However, when I add a ".diff" or ".patch" to the end of the URL, I get something different:

https://github.com/Labutin/CompareBranches/compare/branch_01..branch_02.diff

This is unexpected. My expectation is that adding .diff or .patch to the URL should render the same content as the GUI version (without .diff and .patch on the URL), just formatted to be computer-readable instead of human-readable.

GitHub Support reply:

Hey Shane,

Thanks for reaching out!

We've passed this onto to our Engineering Team to consider.

I'm not sure if or when we'd add the two dot diff for patches but we've for sure pass it on!

Cheers Stacey

🌟Find answers to common questions and learn with other GitHub users in our new GitHub Community Forum 🌟

hugovdm commented 3 years ago

If the pr-check can tell whether there was a pre-squash approval and mention that in the notice, that would help against changes accidentally sneaking in due to an unapproved commit pushed after an approved commit and then all commits squashed together.

markusicu commented 3 years ago

Editing the commit message should disqualify the auto-re-approval. We (should) review commit messages just like code changes.