Pull Request Process

Owned by Laney McGlohon (Unlicensed)
Sept 11, 2017
2 min read

When people put in a pull request, the assumption is that the submitter believes they have done all that is required to fix or implement the issue.

So, when reviewing a pull request, these are the steps to take:

  • Review the issue being fixed to make sure you understand what is being worked on. When reviewing a pull request, please take the time to review the changes and get a sense of what is being changed. If you need more information, ask before the review.

The key things to focus on are:

      • Are the PR description and commit messages clear?
      • Do the functional code changes match the PR description?
      • Does the PR contain tests for new features or bugfixes? Not all PRs require tests, such as wording changes or simple refactoring.
      • Does the commit contain more than it should? 
      • Are two separate concerns being addressed in one commit?

  • Once you understand the issue, look at the code modifications and additions in the pull request. If you have questions or comments for the pull request submitter, put them in the pull request and tag the submitter so they will be notified.

When reviewing pull requests:

      • Give discussion time to settle down. When there is a contentious discussion, please allow 24 hours before merging to make sure everyone's had a chance to respond.
      • It is considered "poor form" to merge your own request, and you should be cautious when merging pull requests from other developers at your own institution.
      • If you are uncertain, bring other contributors into the conversation by creating a comment that includes their @username.
      • If you like the pull request, but want others to chime in, create a +1 comment and tag a user.
      • If you merge the pull request, you should delete the corresponding branch from the archivesspace GitHub repository
  • When you believe the pull request is ready to go into the core code master branch, go ahead and merge the pull request. As a member of the core committers group, you have the authority to merge into the core code base.

  • Once the request has been merged, if the branch attached to the pull request is in the archivesspace GitHub repository, please delete the branch using the delete button that should be provided in the pull request after merging.

 

At any time during the review of a pull request, please feel free to ping other members of the core committers group if you have questions or would like another set of eyes to look at the implementation in the pull request.