Skip to content

Protected Branch Pull Request Management and Merge Process

Matt King edited this page Sep 23, 2019 · 4 revisions

Overview

This documentation is for APG editors with permission to merge into protected branches of the repository.

When there are multiple editors managing pull requests and planning to merge content into protected branches, it is important they coordinate with one another to avoid unnecessary thrashing. Editors will discuss merge plans in advance via meetings or email. And, we'll manage and merge pull requests using the process documented below.

The objectives of the protected branch pull request management and merge process are to ensure:

  1. Continuous quality improvement of the APG.
  2. Ease of understanding and tracking changes, e.g., logs are easy to understand without deep diving into diffs.
  3. Well-scoped pull requests, e.g., the purpose is singular and easily understandable from a 5 to 7 word summary.
  4. Consistent and easy traceability of changes, e.g., pull requests and related issues are consistently linked to relevant commits.
  5. Smooth operations that avoid unnecessary merge conflicts.

Note: merges into unprotected feature branches need not follow any of the conventions documented on this page.

1 - Facilitate pull request title consistency

Title pull requests consistently to facilitate easy scanning through the haystack of changes. Use one of the below described pull request title formats.

Because we haven't yet implemented templates to guide collaborators, this will sometimes mean editing their titles.

2 - Ensure pull request has relevant labeling and issue linking

Most of the time, pull requests have a related issue. It is important that the related issue is referenced from the pull request, ideally in the top comment. Since the related issue gets labeled, is tracked in a milestone, and may also be included in a project, do not duplicate the labeling, milestone association, or project relationship in the pull request; let the issue be the primary tracking entity.

Occasionally, pull requests do not have a related issue. In these cases, adding appropriate labels and a milestone improve traceability. And, if the request is related to a design pattern implementation example, adding it to the project for that example can also be helpful.

3 - Ensure relevant reviews are complete

  1. Make sure that the list of reviews needed is correct, per the pull request review process.
  2. Make sure all reviews are complete.
  3. Make sure all reviews requesting changes have had appropriate follow up.

4 - Prefer rebase for keeping compare branch current

When the compare branch of a pull request has significant history, it can help reviewers better understand the scope of changes if the history is not cluttered with unrelated commits from the base branch. To avoid this problem, use rebase to keep a compare branch current with its base. Because we have required checks on the pull requests, compare branches must be current before they can be merged.

Rebasing is the responsibility of the submitter. However, we may sometimes want to accelerate reviews by rebasing on behalf of the submitter if the compare branch is in the W3C repo. To do it safely, you need to push with the --force-with-lease option:

git checkout compare-branch-name
git pull
git rebase origin/master
git push --force-with-lease

If the compare branch is in a fork and the submitter is not available to update it, the only efficient option is to use the "Update Branch" button in the GitHub UI to updae the compare branch, which will merge the base branch into the compare branch. Alternatively, change the base to a new feature branch, merge (without squashing) into the feature branch, and open a new PR with the feature branch as the compare branch and the original base. This is a last resort because the contributor will not be creditted with a merge to the default branch.

If all reviews are complete, you know all checks will pass, and the compare branch is ready for merge, then it really doesn't matter if you merge or rebase to update the compare branch because its commits will all get squashed anyway.

5 - Use squash merge method

We don't want the history of the protected branches cluttered with all the individual commits from each pull request. None of that history is useful given the nature of the APG and the way we manage it. By having a clean history in the publication branches, it is easy for both editors and typical users of the APG to understand the history of APG changes.

So, when merging a pull request into a protected branch, be sure the merge method option is set to squash. This will give you the opportunity to write one clean commit statement (as described below) to describe all the work in the pull request.

6 - Use APG commit title conventions

Commit titles in the publication branches are formatted as headings in a detailed change log that gets published on the wiki (see example change log). The detailed change logs are referenced from the change summary sections of the appendix. The goal is to make it easy for readers to know what was changed, how it was changed, and quickly discern whether or not they want to dig deeper into a specific change.

When squashing, use one of the title formats described below.

7 - Use APG commit description conventions

Occasionally, the commit title says all that needs to be said. But, most of the time, it is beneficial to include a detailed description of changes. In some cases, the description should also include the rationale for the choice of implementation approach.

If the commit has related issues, always include references to the related issues with #123 where 123 is the issue number. These will be links when viewing the commit on GitHub.

Typical format is a phrase referencing the issue followed by a list of the changes, e.g.:

Resolves issue #123 by:

  • Description of change 1.
  • Description of change 2.

Here are some examples of commit descriptions.

For a commit titled, "Date picker dialog example: Fix aria-selected bug -- issue 1072 (pull #1074)":

Resolves issue #1072 so behaves correctly if the first day of the month is typed into the textbox and then the choose date button is pressed.

For a commit titled, "Add datepicker based on dialog pattern for issue 1046 (pull #1054)":

To resolve issue #1046, adds a modal dialog example that implements a date picker. The modal dialog includes a calendar grid that implements the grid pattern.

For a pull request titled, "Image Carousel Example: Use buttons for controls and make rotation control always visible (pull #1018)":

Resolves issue #1007 by:

  • making the stop/start rotation button always visible.
  • Using button elements instead of links for the next and previous slide controls.

Also:

  • Allows user to change between 2 view options: one with controls and captions overlayed on the images and one with them outside the image frames.
  • Improves documentation in the accessibility features section.
  • Adds regression tests.

Pull Request and Commit Title Formats

We have 3 broad categories of pull requests:

  1. Part scope: The request scope is limited to a specific part (e.g., section or example page) of the APG. It may include many kinds of changes to resolve one (ideally) or more issues with that part.
  2. Change scope: The request scope is defined by a single specific change that is made to multiple parts of the APG. In general, more narrowly targetted part-scoping is prefferable. However, if the change is very narrowly defined, e.g., change all occurrences of string x to string y, and needs to be made in many parts, then change-scoping is appropriate.
  3. Addition scope: This is basically a part-scoped request except that instead of revising existing content, it is adding a completely new part (e.g., section, example page, capability, etc.).

We have pull request and commit title formats for each type of request.

Part scope titles:

  • request format: Part_Name: Change_Summary for issue 123
  • Commit format: Part_Name: Change_Summary (pull #123)

Change scope titles:

  • Request format: Change_Type: Change_Summary for issue 123
  • Commit format: Change_Type: Change_Summary (pull #123

Addition scope titles:

  • Request format: Add Part_Description for issue 123)
  • Commit format: Add Part_Description (pull #123)

The title should be describing what the PR does, not what the contributor did. Keep titles relatively short -- in the neighborhood of 60 characters. However, a longer title is better than a mysterious title.

Note that the issue number phrase in request titles is optional -- some pull requests do not have associated issues.

See below for additional information and examples for each title format.

Titling part-scoped requests and commits

Pull request title format:

Part_Name: Change_Summary for issue 123

Example pull request titles:

  • Combobox Pattern: Revise Escape behavior for issue 1066
  • datepicker dialog: hide disabled dates for issue1121
  • Naming and Describing Section: Revisions to address feedback in issue 1050

Commit title format:

Part_Name: Change_Summary (pull #123)

Example commit titles:

  • Spin Button Pattern: Add aria-invalid guidance for values outside allowed range (pull #825)
  • Image Carousel Example: Use buttons for controls and make rotation control always visible (pull #1018)
  • Regression Tests: Add data-test-id attribute to 8 example pages (pull #810)

Titling change-scoped requests and commits

The first element in the title format is a change_type identifier. Change type identifiers include:

  • Editorial: A content revision to improve readability or consistency with W3C or APG editorial guidelines.
  • Chore: A content revision to otherwise improve content quality, e.g., fix HTML errors, source formatting problems, etc. These types of changes are often invisible to people reading the APG.

For change-scoped pull requests, It is generally helpful for the commit to include a detailed description that describes which parts were revised.

Pull request title format:

Change_Type: Change_Summary for issue 123

Example pull request titles:

  • Editorial: Remove outdated links to feedback issues
  • Chore: Fix NU Checker Validation Errors

Commit title format:

Change_Type: Change_Summary (pull #123

Example Commit titles:

  • Editorial: Fix multiple instances of repeated words (pull #867)
  • Chore: Fix NU Checker Validation Errors (pull #820)

Titling addition-scoped requests and commits

Pull request title format:

Add Part_Description for issue 123)

Example pull request titles:

  • Add disclosure menu example for issue 1028
  • Add special acknowledgements section to appendix
  • Add tests for radio/radio-2/radio-2.html

Commit title format:

Add Part_Description (pull #123)

Example commit titles:

  • Add section on accessible names and descriptions (pull #994)
  • Add carousel pattern (pull #957)
  • Add disclosure menu example for issue 1028 (pull #1070)
Clone this wiki locally