I have observed a pattern where authors force-push their changes during every review iteration, so that a pull request always contains one commit. This creates the following problems:
1. It is hard to see what has changed between review iterations. 2. Sometimes authors make changes in parts of pull requests that the reviewer did not comment on, and such changes may be unnoticed by the reviewer. 3. After a force-push, comments made by reviewers on earlier commit are hard to find. A better workflow may be to: 1. Between review iterations authors push changes in new commit(s), but also keep the original commit. 2. If a follow-up commit does not constitute a meaningful change of its own, it should be prefixed with "fixup: ". 3. Once review has finished either: - Authors squash fixup commits after all reviewers have approved the PR per request of a reviewer. - Committers squash fixup commits during merge. I am curious what thoughts or suggestions others have. In particular: 1. Should we document guidelines for iterating on PRs in our contributor guide? 2. Is it acceptable for a reviewer to ask the author to rebase squashed changes that were force-pushed to address review feedback onto their original commits to simplify the rest of the review? Thanks. Related discussion: [1] Committer Guidelines / Hygene before merging PRs https://lists.apache.org/thread.html/6d922820d6fc352479f88e5c8737f2c8893ddb706a1e578b50d28948@%3Cdev.beam.apache.org%3E
