Merging a pull request for PnP core team members
This wiki page provides guidance for the PnP core members to help them with processing pull requests.
When a contributor creates a pull request this is adding the queue of pull requests on the github site. Core members will receive a mail notification when this happens. The pull request queue on github can be found here: https://github.com/OfficeDev/PnP/pulls
When there's a request open you can click on it to see the details of the pull request. Typically the author has described what the pull request is meant to do which gives you a first impression. When you think you can process this pull request you should first check the Commits and Files changed tabs. Often you can already determine whether this pull request can be further processed or requires changes. If for example the contributor created the pull request against the master branch then you'll see a pull request with lots of files which should be rejected.
After this first analysis you'll need to provide feedback to the contributor. Either you tell that the pull requests will be processed or you explain why the pull request cannot be merged.
The fact that you've written a note in the pull request typically also means that you'll process the request. Alternatively you can also assign yourselves to the request
When you merge a pull request you've two options:
- Simple pull requests (a new unit test, a typo change, new documentation,...) often can be merged directly from within the github site. This however will only apply to a minority of the pull requests.
- All other pull requests first need to be merged into your local repository, tested, updated if needed and only then they can be merged to the dev branch.
In this guide we assume we're dealing with a pull request that needs to be merged. To merge this pull request you'll need to use the git command line. This can be done from the command prompt inside Visual Studio 2013 or any other git commandline. Many of the core team members use Atlassian SourceTree, so for this guide we'll use that but the same can be done using Visual Studio 2013.
First we'll need to make sure that you can pull the changes associated with the pull request. To do so we'll setup the contributor's repository as a remote repository. Doing this requires you to know the url to the contributor's repository. Easiest way here by clicking on "command line" in the pull request as this will show the url:
Copy the url and open a command prompt. Ensure you're on the dev branch. You can ensure this by typing:
git checkout dev
Typically this is also indicated command prompt:
Next step is adding a remote repository to your forked repository.
git remote add contributor <url to contributors git repository>
git fetch contributor
At this point you can merge a branch from the contributor's repository with your dev branch. Since a pull request always contains all changes for a single branch this means that merging the remote branch will pull in all changes from the pull request. However before you do the merge it's important that:
- You've pulled the latest changes from the main PnP dev branch to your local dev copy: See Refreshing your forked repository with changes done in the main PnP repository for the Visual Studio 2013 instructions. When you're using Atlassian SourceTree you can open the repo and use the fetch and pull buttons to achieve the same.
- You know which branch the contributor has used to create the pull request. This can be found on the github site: in our sample the used branch was the master branch.
To merge the branch use the following command:
git merge contributor/<branch name>
At this point the branch is merged into your dev branch and you can again remove the remote branch:
git remote rm contributor
Depending on the change it might be better to test the change first in a separate branch. If so then please follow below instructions to merge the contributors changes:
git checkout -b pr<pr number> dev
git merge contributor/<branch name>
Now you're switched to a pr branch (e.g. pr162) that was branched of dev and contains the contributors changes. Once you've tested you'll need to switch to the dev branch, merge your changes back to the dev branch and delete the pr branch:
git checkout dev
git merge pr<pr number>
git branch -d pr<pr number>
Below console output shows the complete process:
todo
Before you commit the merged pull request you'll to verify that everything works. Basic tests are:
- Ensuring the code compiles
- Ensuring that unit tests run fine
- Opening up the impacted files to see they're correctly merged, have proper documentation,...
In this guide we're going to add some changes on top of the pull request, more in particular we're restructing the documentation to be in line with our guidance:
Once you're done testing you need to commit your additional changes. Easiest is right clicking the solution/project in Visual Studio 2013 and choosing Commit.... Provide a comment to describe the commit and press "Commit".
Pushing the merged pull request changes and your additions to the dev branch of the main PnP repository
Once the commit is done it's only committed to your local repository, not to the repository hosted on the github server.
To push the merged pull request changes and optionally the additions you did you can either user Visual Studio (see Make changes to your local copy of the forked PnP repository) or use Atlassian SourceTree:
By pushing the changes to the server github automatically detects that the commits belonging to the pull request have been merged and as such it will automatically mark the pull request as merged and close it.
The fact that github closes the pull request will also trigger a mail notification to the contributor, but it's best to also provide a thank you and possible explanation of done changes:
