Skip to content

Commit

Permalink
Meta: Change PR checkout steps
Browse files Browse the repository at this point in the history 
Updates the guidelines for editors with the new workflow made possible
by the GitHub feature change that gives repo maintainers write access to
contributor’s PR branches.

Fixes whatwg#1943.
  • Loading branch information
@sideshowbarker
sideshowbarker authored and Alice Boxhall committed Jan 7, 2019
1 parent c6501a5 commit 22bcc74
Showing 1 changed file with 39 additions and 44 deletions.
83 changes: 39 additions & 44 deletions TEAM.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,69 +10,62 @@ For normative changes, ask for a [web-platform-tests](https://github.com/w3c/web

If a follow-up issue is filed, add the `html` label.

### Fetching and reviewing pull requests from forks
### Checking out pull requests from forks

Pull requests from external contributors come from their forks. To be able to more easily review the commits in those pull requests, you can optionally configure your clone such that:
Pull requests from external contributors come from branches in their forks. You can check out those external branches in order to review and test the commits in those pull requests, and to be able to push changes to them on your own (e.g., fixes for typos)—rather than needing to write review comments asking the PR contributor to make the edits.

* Alongside the remote branches you already have [created by the team](https://github.com/whatwg/html/branches), you'll also have remote branches for all existing PRs.
* Thus you can `git checkout` any PR branch you want to build/review, and use `git pull` to pull any updates to it.
To checkout a PR branch, note the user it's coming from and the branch they used in their fork. For example, for user `estark37` with branch `example-fix`, you would do

To do all that, use these steps:

1. Do one of the following:
* Run the following command to **globally configure, for all repositories you pull from**, automatic fetch of branches for PRs from forks:

```bash
git config --global --add remote.origin.fetch "+refs/pull/*/head:refs/remotes/origin/pr/*"
```
That will add the following two lines to your `$HOME/.gitconfig` file:

```
[remote "origin"]
fetch = +refs/pull/*/head:refs/remotes/origin/pr/*
```

If you change your mind later about globally enabling that behavior, you can disable it by removing those lines.
```bash
git remote add estark37 https://github.com/estark37/html.git
git fetch estark37
git checkout -b estark37-example-fix estark37/example-fix
```

* Alternatively, to enable automatic fetch of branches in PRs from forks **just for this repo**, omit `--global` from the above command.
You can then push to the `estark37-example-fix` branch and it will update the `example-fix` branch in `estark37`'s fork, and thus will update the pull request.

2. Run `git fetch` or `git pull` to do the initial fetch of all branches for current PRs.
#### Git config tweak

3. Run `git checkout pr/NNN` to check out a particular PR branch. For review purposes, you may want to subsequently do `git rebase master` to make sure it is on top of the latest changes from `master`.
It's recommended that you also make the following change to your `git` configuration:

4. If a contributor subsequently pushes changes to the corresponding branch for that PR in their fork (for example, in response to your review comments), then: make sure you're on the checked-out `pr/NNN` branch locally, and reset to the latest from the remote, by doing the following:
```bash
git config push.default upstream
```

```bash
git checkout pr/NNN
git fetch
git reset --hard origin/pr/NNN
```
If you make that change, then whenever you're in a local PR branch and want to push changes back to the corresponding external branches, you can just run `git push` with no arguments (rather than also needing to specify the remote name and branch name as arguments). Otherwise, you need to also specify the remote name and branch name each time you push.

### Merging pull requests from forks
If you want to enable that same ability for all your project clones, also specify the `--global` option: `git config --global push.default upstream`.

Just use the normal green button (labeled **Merge pull request**) in the pull-request page in the GitHub Web UI. After you press that, another green button will appear, labeled **Confirm squash and merge**. Ensure that the commit message follows [the guidelines](https://github.com/erlang/otp/wiki/Writing-good-commit-messages), then press that and all the commits from that PR branch will be combined into one commit to the master branch.
#### Helper script

Otherwise, if you want to merge a PR from the command line, you can, once you have completed the **Fetching and reviewing pull requests from forks** setup (above), by doing the following:
You can add the following helper script to your `.bash_profile` or similar to make the process above slightly simpler:

```bash
git checkout pr/NNN
git rebase master
... build and review the spec ...
git checkout master
git merge pr/NNN --ff-only
checkout-pr() {
local REPO=`basename $(git config remote.origin.url | cut -d: -f2-)`
local REMOTE_URL=https://github.com/$1/$REPO
if [ "`git config remote.origin.url | cut -d: -f1`" == "git@github.com" ]; then
REMOTE_URL="git@github.com:$1/$REPO"
fi
git remote add $1 $REMOTE_URL 2> /dev/null
git fetch $1
git checkout -b $1-$2 $1/$2
}
```

This checks out the PR's commits, rebases them on `master`, then fast-forwards `master` to include them.
You can then use it as

Before pushing, you should amend the commit message with a final line containing `PR: https://github.com/whatwg/html/pull/XYZ`, so that we can easily see a link back to the pull request's discussion. Finally, you can do `git push origin master` to push the changes. Don't forget to comment on the pull request with something like "Merged as 123deadb33f" before closing.
```bash
checkout-pr estark37 example-fix
```

### Merging pull requests from branches
### Merging pull requests into master

Pull requests from other editors or members of the WHATWG GitHub organization may come from branches within this repository.
Just use the normal green button in the pull-request page in the GitHub Web UI, but first ensure the commit message follows [the guidelines](https://github.com/erlang/otp/wiki/Writing-good-commit-messages).

Just as with PRs from forks, you can merge PRs from branches in this repo to the master branch just using the normal green button (labeled **Merge pull request**) in the pull-request page in the GitHub Web UI. After you press that, another green button will appear, labeled **Confirm squash and merge**. Press that and all the commits from that PR branch will be combined into one commit to the master branch.
#### Merging to master from the command line

Otherwise, if you want to cleanly merge a PR from a branch within the repo using the command line, you need to add an extra step in addition to the steps you'd follow for merging a PR from a fork. These are the steps:
Regardless of whether a pull request comes from a contributor (in which case the branch is from a different remote repository and you've already used the **Checking out pull requests from forks** steps above to fetch the branch) or from other editors or members of the WHATWG GitHub organization (in which case the branch is within this repository), the steps for cleanly merging it to master are the same:

```bash
git checkout BRANCH_NAME
Expand All @@ -83,7 +76,9 @@ git checkout master
git merge BRANCH_NAME --ff-only
```

The additional `git push --force` line here ensures that the original branch gets updated to sit on top of `master` as well. This ensures GitHub can automatically figure out that the commits were merged, and thus automatically close the pull request with a nice purple "merged" status. So at this point you can do a `git push origin master` to push the changes, and GitHub will close the PR and mark it as merged.
This checks out the PR's commits, rebases them on `master`, then fast-forwards `master` to include them.

The `git push --force` line here ensures that the original branch gets updated to sit on top of `master` as well. This ensures GitHub can automatically figure out that the commits were merged, and thus automatically close the pull request with a nice purple "merged" status. So at this point you can do a `git push origin master` to push the changes, and GitHub will close the PR and mark it as merged.

## Bugs

Expand Down

0 comments on commit 22bcc74

Please sign in to comment.