Check for commits from others in PRs #246
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This PR adds a new flag `checkForCommitsFromOthers` that when enabled (false by default) will check if the last commit on the PR is from the same committer as that of the local commit. This is useful for scenarios when either multiple people collaborate on the same PR, or when the CI is set up such that it automatically adds a code formatting commit if needed. Currently in such scenarios it is very easy to run `spr diff` and lose all of the work from others that was added to the PR since you ran `spr diff` the last time. Test Plan: I tried running `spr diff` in a repo where CI automatically pushes a commit to fix code formatting. The check was triggered. The first time I followed the instructions with `git merge`, the second time I typed "yes". (In my test case the `git merge` did not actually work because the the automated CI commit was just removing the trailing whitespace I added in my test PR, thus making the whole PR empty. Therefore the `git merge` locally did not trigger a merge conflict, nor did it remove the whitespace I had in my local commit. Maybe a different merge strategy is needed? Or maybe the extra commits from the PR branch need to be squash-rebased into the local commit? Open to suggestions)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds a new flag
checkForCommitsFromOthersthat when enabled (false by default) will check if the last commit on the PR is from the same committer as that of the local commit.This is useful for scenarios when either multiple people collaborate on the same PR, or when the CI is set up such that it automatically adds a code formatting commit if needed. Currently in such scenarios it is very easy to run
spr diffand lose all of the work from others that was added to the PR since you ranspr diffthe last time.Test Plan:
I tried running
spr diffin a repo where CI automatically pushes a commit to fix code formatting. The check was triggered. The first time I followed the instructions withgit merge, the second time I typed "yes".(In my test case the
git mergedid not actually work because the the automated CI commit was just removing the trailing whitespace I added in my test PR, thus making the whole PR empty. Therefore thegit mergelocally did not trigger a merge conflict, nor did it remove the whitespace I had in my local commit. Maybe a different merge strategy is needed? Or maybe the extra commits from the PR branch need to be squash-rebased into the local commit? Open to suggestions)