I don't think it's actually harder to roll back a set of commits that are merged together. Git has the notion of first-parent commits (you can see, for example, "git log --first-parent", which filters out the intermediate commits). In this sense, PRs still get merged as one unit and this is preserved even if intermediate commits are kept. Perhaps the bigger issue is that we need better documentation and a playbook on how to do this these common tasks in git.
On Wed, Sep 19, 2018 at 9:27 AM Thomas Weise <t...@apache.org> wrote: > Wanted to bring this up as reminder as well as opportunity to discuss > potential changes to our committer guide. It has been a while since last > related discussion and we welcomed several new committers since then. > > Finishing up pull requests pre-merge: > > https://beam.apache.org/contribute/committer-guide/#finishing-touches > > PRs are worked on over time and may accumulate many commits. Sometimes > because scope expands, sometimes just to separate independent changes but > most of the time the commits are just fixups that are added as review > progresses. > > It is important that the latter get squashed prior to PR merge, as > otherwise we lost the ability to roll back changes by reverting a single > commit and also generally cause a lot of noise in the commit history that > does not help other contributors. To be clear, I refer to the "Fixup!", > "Address comments", "Address even more comments" type of entries :) > > I would also propose that every commit gets tagged with a JIRA (except > those fixups that will be squashed). Having the JIRA and possibly other > tags makes it easier for others not involved in the PR to identify changes > after they were merged, for example when looking at the revision history or > annotated source. > > As for other scenarios of jumbo PRs with many commits, there are probably > situations where work needs to be broken down into smaller units, making > life better for both, contributor and reviewer(s). Ideally, every PR would > have only one commit, but that may be a bit much to mandate? Is the general > expectation something we need to document more clearly? > > Thanks, > Thomas > >
