Merging Pull Requests

The other side of the coin is reviewing and testing submitted pull request so they can be merged. This is usually done by by a maintainer of the openwebwork repository, but it doesn't have to be. Anyone can review a pull request. Something to keep in mind is that anything pulled into master is immediately distributed and has to be rock solid. Anything pulled into a release/x.y branch will be merged into master in about six months and needs to be very stable. Things merged into develop will make it into master in about a year or so, passing through a release branch on the way. In particular something pulled into develop will eventually end up in master, and should be at least in its "beta" stage. Experimental or preliminary code should be kept on individual contributors personal github repos and distributed from there.

The standard procedure follows:

1. Open the pull request and check to see that the file changes look sane and that the feature is being pulled into the correct branch.

Note: First time submitters don't always use feature branches. Often they are submitting their personal versions of develop. As long as the file changes look fine its reasonable to think of "develop" as a badly named feature branch. However you should point them to this documentation for future contributions.

2. Open the "Network" page for openwebwork and find the line corresponding to the feature branch on the Network page. (You may need to click "refresh".) Ideally the line will either be a "loop" or a "ladder" minus the final pull. (See the diagrams in Coding and Workflow).

   • The branch must track (i.e. be split from) the same branch it is being pulled into. E.G. If it splits off master it cannot be pulled into develop. Note: This is a common issue with first time submitters. If its a problem, work with them and point them to these instructions. They can salvage their work by creating a proper feature branch and then either rebasing or using cherry-pick to move their commits to the feature branch.
   • If a branch which is targeted for, say, develop, has had master or release/x.y merged into it, then it cannot be merged. The developer will need to make a new feature branch tracking the appropriate branch and then use rebase or cherry-pick to move their code over.
   • Beware of spaghetti pull requests. Its fine if two feature branches which both track the same branch in origin are merged together, but it creates confusion. In particular, if a feature branch tracking master has been pulled into a feature branch tracking develop then the feature branch tracking develop cannot be merged into openwebwork.

3. Get a local copy of the proposed changes. The easiest way to do this is to go to the bottom of the "Conversation" tab on the pull request page, click the "command line" link, and run the commands under "Step 1". You will need to add "origin/" in front of the target branch. The result will look something like

git checkout origin/<target-branch>
git pull https://github.com/<git-username>/webwork2.git <feature-branch-name>

Note: This code checks out origin/<target-branch> as a headless branch (i.e. no -b branch-name). This works since you won't need to keep the pull request changes permanently. However, you can checkout a permanent testing branch if you want. Next, restart the webserver, and update config files or upgrade databases as necessary.

4. Test the code using the testing instructions provided in the pull request. If they didn't provide instructions, figure out your own way to test the changes. If/When something breaks, report it as a comment. The submitter can fix the bugs and the pull request will update automatically.

5. Do a more thorough overview of the file changes. Check to see that the changes look reasonable and that nothing seems unusual, out of place, or wrong.

6. Continue to try and break the pull request. When the code is ready to be merged, write a short comment explaining what you have tested and merge the commit. You should merge the pull request using the web interface, not via the command line.

7. If the pull request is a hotfix for master you should also test and merge the associated pull request into release/x.y. (If there isn't an associated pull request you can create one yourself by visiting the developers personal repository and following the usual process.) You may need to ask the developer to pull release/x.y into the feature branch and fix conflicts, but this should be done after you have merged into master.
   Note: this is one of the only times its OK to pull a feature branch into something other than its tracking branch.

If you are interested in reviewing and merging pull requests you should know that you do not need to be a maintainer to do so. Just follow steps 1 through 6 and when you are done write a comment explaining what you have tested and what the results were. A maintainer can then merge the request later.