Conversation
block-commits/pullRequestChecker.js
Outdated
| } | ||
| if (this.actionFixup == 'request-changes' || this.actionMerge == 'request-changes') { | ||
| const okMessage = 'PR no longer has any merge or fixup commits.'; | ||
| const failPrefix = 'PR requires a rebase. Found: '; |
There was a problem hiding this comment.
We should be careful requiring rebase.
whenever we do so, we should instruct to use
git rebase -i $(git merge-base master head)
There was a problem hiding this comment.
Would you like me to add that to the message? Or can you suggest a better phrasing to avoid suggesting a rebase?
There was a problem hiding this comment.
For merge commits the current wording is fine.
for fixups you can omit that for now. we can improve the wording once we start using this and can see this in practice
There was a problem hiding this comment.
I reworded both the ok and fail messages. PTAL.
hashhar
left a comment
hashhar
left a comment
There was a problem hiding this comment.
style comments % !amend and last comment about bot comment getting buried
|
Reminder to autosquash. 😉 |
|
We're not updating existing reviews for this reason, but adding new
ones,without repeating the last one.
|
|
Then I worry about noise. But let's proceed with this and we can address feedback as it starts getting used. LGTM from me % autosquash. @findepi PTAL if you have remaining comments. |
nineinchnick
force-pushed
the
block-commits
branch
from
47774b2 to
0458700
Compare
nineinchnick
force-pushed
the
block-commits
branch
from
0458700 to
10ce549
Compare
3 participants
To be used in trinodb/trino#13494