Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc: document use of the Squash and merge button #11686

Closed
wants to merge 1 commit into from
Closed

doc: document use of the Squash and merge button #11686

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 7 additions & 10 deletions COLLABORATOR_GUIDE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,16 +302,13 @@ The CTC should serve as the final arbiter where required.

## Landing Pull Requests

* Please never use GitHub's green ["Merge Pull Request"](https://help.github.com/articles/merging-a-pull-request/#merging-a-pull-request-using-the-github-web-interface) button.
* If you do, please force-push removing the merge.
* Reasons for not using the web interface button:
* The merge method will add an unnecessary merge commit.
* The rebase & merge method adds metadata to the commit title.
* The rebase method changes the author.
* The squash & merge method has been known to add metadata to the
commit title.
* If more than one author has contributed to the PR, only the
latest author will be considered during the squashing.
If the Pull Request can be landed as a single commit, you can use GitHub's green
["Squash and merge"](https://help.github.com/articles/about-pull-request-merges/#squash-and-merge-your-pull-request-commits)
button, as long as you:
* Remove the PR number, e.g. `(#1048)`, from the commit message title.
* Ensure that the commit message is correct, remove any fixup commit messages.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps just say that the commit message needs to be updated to conform to the contributions guidelines?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree.

* Make sure CI was run on the latest update to the Pull Request.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this something specific to using the button?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@seishun in a certain sense it is as you shouldn't press the button without making sure that CI is passing. I would keep it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lpinca you should also not press the button until someone has reviewed the PR. Should this be mentioned here too?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think not, that is already known by collaborators and is part of the on-boarding process. You can argue that landing a PR after ensuring that CI is passing is also part of the on-boarding process but I would still keep this sentence as it doesn't harm and is not off-topic.

This is just my opinion though.

Copy link
Member

@lpinca lpinca Mar 4, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@seishun also that is "included" in the next line:

Add the correct metadata (see below).

That means that PR must be reviewed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to give a complete list, as I assumed someone using the button to make life easier wouldn't necessarily want to read through the command line instructions to work out which rules still applied.

* Add the correct metadata (see below).

Always modify the original commit message to include additional meta
information regarding the change process:
Expand Down