Pull Requests - Atlantic57/style-manual GitHub Wiki

Overview

Pull requests should be created any time new features or bug-fixes require merging into a working build. As a general rule, a PR should be limited in scope and address an issue or small group of related issues rather than an entire epic. By keeping scope limited we can make sure that merging is easier and the reviewer isn't overwhelmed by the assignment. It's better to have many small PRs as opposed to fewer, larger ones.

Submit Requirements

At minimum, a PR should contain:

  • Links to the issues that it resolves. Cite the issue number in the reference. A helpful convention is to use Github checkboxes next to the reference, allowing the reviewer to check items as complete when they have passed review.
  • Links to the environments that the reviewer and subsequently QA resource would use to validate the feature or fix.
  • Assignment to a senior dev resource and, optionally, the project manager

If helpful or relevant, a PR may also contain:

  • Steps required to validate, if not explained in the referenced issue
  • Screenshots that support/validate the fix
  • Any notes related to deployment nuances and timing

Review Requirements

If assigned a PR, our goal is to review as soon as possible, ideally no later than one working day. We have integrated the Pull Reminders Slackbot into our workflow to send repeat reminders to the team and assignee when PRs are lingering.

In general, a reviewer should comment on:

  • Validation, does the proposed fix fulfill all requirements?
  • Code quality: does the submitted code meet our standards?

If a PR is not approved:

  • Provide sufficient reason as to why, letting the submitter know what issues exist
  • Provide steps required to reproduce the error you are seeing, if not immediately obvious
  • Provide images or screen caps of issues that persist if it helps illustrate
  • Leave notes on a line-by-line basis where applicable, with an @mention to the submitter
  • Pertinent emoji and/or animated gifs to adequately express emotion

Submit Example

Creating Login

Resolves

  • #100 - Create login screen
  • #101 - Successful user login
  • #102 - User login errors
  • #104 - User reset password

Validate the above on the test environment.

Notes

  • We should incorporate a password strength indicator in the future
  • We still need text for error messages
  • Input limits set at 100 characters per-field

Blockers

  • We should seed test environment with client logins (#106) before we share outside company.

Deployment

  • This should not be deployed until mm/dd/yyyy.