Skip to content

Pull request responses

cgolubi1 edited this page Nov 15, 2014 · 1 revision

When developers submit prospective changes to ButtonMen code, these changes need to be approved before they are merged. Devs submit changes for approval by opening a pull request from their own feature branch to buttonmen-dev/master. Especially for complicated changes, it can take awhile for approvers to review pulls, which may be frustrating.

This is an attempt at an extremely informal SLA for response to pull requests. The intention is to set expectations for devs, and to focus approvers on iterating pulls often and keeping devs informed about the current state. This should be read as an ideal - what we're aiming for as approvers - it is not a promise. We're all volunteers, and life can and does intervene.

When a new pull is submitted, we'll aim for the following response:

  • Within 4 days of submission, do an initial once-over of the pull. We'll spend 30 minutes or less on this, and the goals, in priority order, are:
    1. Look at the pull, figure out what we want to do before approving it, and update the ticket to say so. Things we might do before approving will be a subset of:
      • read the code carefully
      • run a few simple tests
      • run complex tests (and possibly write new tests)
      • discuss a design decision
      • have someone other than the primary approver read the code/run tests
    2. Kick the pull back to the requester with things that definitely need to be fixed, e.g. obvious major commit errors, broken/missing jenkins runs. State explicitly whether we'll do some testing anyway or wait for those fixes before testing.
    3. If the needed testing is really simple, and time permits, go ahead and do it. (But there's no guarantee of this, even for simple pulls.)
  • Within 8 days of the initial response, spend up to two hours on the pull, working through as much of the identified tasking as fits in that time. Update the ticket with current state and next steps at the end of that period.
  • If the pull is still on our plate after that, there's no explicit promise about time spent, but we'll update the ticket once a week with current state and next steps.

General comments:

  • Any time an approver updates a pull ticket, state explicitly what the next step is and whose plate it's on (the approver, the dev, someone else)
  • When a pull is updated with changes, treat it like a new pull --- initial response within 4 days, two hours of work within 8 days after that.

Clone this wiki locally