Skip to content

Checklist For Merging A Pull Request

Casey Carter edited this page Oct 8, 2021 · 38 revisions

Always

  • Merge GitHub and MSVC PRs at the same time, after both of their tests have passed.
    • When mirroring multiple GitHub PRs, the MSVC PR should be a semi-linear merge. Any NON-GitHub changes should be separate commits in the MSVC PR.
    • When updating stl/debugger/STL.natvis, simultaneously update VS's src/vc/CppEE/src/Visualizers/STL.natvis.
  • Verify that the PR is linked to the issues that it resolves.
    • When linking issues using keywords, remember that the Words Of Power are important and they must be repeated when closing multiple issues.
    • Additionally, tag closed issues as fixed. Leave the other tags unchanged. We use fixed to mark issues that were solved via commits, as opposed to being resolved without a commit.
    • If any DevCom/VSO bugs were linked, resolve them appropriately. Reply to customers on DevCom, and set the Release/Milestone/Target fields in VSO.
  • Proofread the commit message.
    • The title should be clear, i.e. suitable for scanning in git log, gitk, etc.
    • The body can be empty (i.e. the PR number records what happened and why), or it can contain a small or large amount of additional information. However, it shouldn't be a messy concatenation of commit messages during development (e.g. "fixed typo", "code review feedback", etc.).
    • Double-check all numbers (e.g. paper numbers and issue numbers) to catch common mistakes like reversing digits.
  • Thank the contributor, especially if they're a first-time contributor! 😸
  • Update the wiki's Changelog.
    • If a primary feature was implemented simultaneously with patch papers and/or LWG issue resolutions, list them as sub-bullets. The patch papers and LWG issues should have been mentioned in the GitHub issue for the primary feature; you can copy and modify the Markdown there.
  • Archive the PR's card which should now be in the "Done" column of the Code Reviews project.

When Adding Features

  • Microsoft-internal: Update stl.xlsx. (Note to contributors: this spreadsheet contains the same information as our cxx20 tagged issues; Excel simply provides an easy-to-scan dashboard.)
    • Change the Status filter to display patch papers; restore this when you're done.
    • Change the Status from missing to the release that this will ship in. (Note that we update the Status for patch papers, but leave their Notes, Dev, and GitHub columns as saying patch.)
    • Update the Notes. We use [GH] for GitHub contributors and [14] (sometimes [17]) for unconditional features.
    • Record the Dev. This is a GitHub or Microsoft username. It should be hyperlinked to the GitHub PR that was merged.
    • Cut and paste the updated rows to sort them by their updated Status (followed by Paper, except that we try to group patches by their primary paper, so this is a manual sort).
    • Note: Certain copy-paste operations seem to clutter up the extensive Conditional Formatting, but cutting-and-pasting rows never seems to be harmful.