Skip to content

Agree on commit reverting strategy #12979

Description

@gibfahn

The COLLABORATOR_GUIDE doesn't seem to cover how to revert commits, and specifically what the commit message should be. I think we should try to agree what the standard process is.

For single commits with git revert HASH:

  1. Leave the commit message as is.
  2. Modify (not least to pass node-validate-commit), e.g. fs: Revert throw on invalid callbacks #12976

For multiple commits with git revert FROM...TO:

  1. Leave as individual commits
  2. Format commit messages, e.g. Revert commits that cause failing tests on Windows CI #4679
  3. Squash the commits

Activity

  1. gibfahn commented on May 11, 2017

    @gibfahn
    MemberAuthor

    FWIW my preference is 1. and 2.

  2. added
    metaIssues and PRs related to the general management of the project.
    on May 11, 2017
  3. evanlucas commented on May 12, 2017

    @evanlucas
    Contributor

    I was under the impression we always used just git revert <sha> and then amended the commit message with the reason for reverting

  4. jasnell commented on May 12, 2017

    @jasnell
    Member

    I don't believe I've ever seen us do a multiple commit revert. My preference is definitely git revert HASH on a single commit at a time with an amended commit message.

  5. gibfahn commented on May 12, 2017

    @gibfahn
    MemberAuthor

    Refs #4679 (comment) from @rvagg :

    FYI, we've consistently used the format: Revert "original commit msg" as reversion messages, so even the subsystem prefix goes into the quotes. The tooling we have recognises this format too.

  6. gibfahn commented on May 12, 2017

    @gibfahn
    MemberAuthor

    Okay, I think we have consensus, so I'll close this as decided. We use git revert, and leave the commits as they are.

    If anyone disagrees then comment/reopen.

  7. joyeecheung commented on May 12, 2017

    @joyeecheung
    Member

    Ah..wait, but this practice is still not documented, no?

  8. gibfahn commented on May 12, 2017

    @gibfahn
    MemberAuthor

    @joyeecheung Good point

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    metaIssues and PRs related to the general management of the project.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions