To be concrete, it is very easy to revert a commit in any case: 1. First, use "git log --first-parent" to find the first-parent commit corresponding to a PR merge (this is a one-to-one correspondence). 2. Use "git revert -m 1 <commitid>" to revert the commit; this selects the first parent as the base for a merge commit (in the case where a single commit needs to be reverted, just use "git revert <commitid>" without the "-m 1" flag).
In any case, as a general good engineering practice, I do agree that it is highly desirable to have small independent PRs instead of large jumbo PRs whenever possible. On Wed, Sep 19, 2018 at 11:20 AM Charles Chen <c...@google.com> wrote: > 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 >> >>
