Add a How to test section to the PR template - #2238
Merged
Merged
Conversation
The template asserts in its checklist that tests passed, but had nowhere for the steps a reviewer follows to see it themselves, so PRs kept inventing the section by hand. Also promotes the collapsed security/performance/breaking-changes block to a normal section. Folding it hid exactly the content that needs the most scrutiny, and on a PR where performance is the headline it forced the author to cover the topic twice: once in the body and once in the fold.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds a
## How to testsection to the PR template, and promotes the collapsed security/performance/breaking-changes block to a normal section.Currently the checklist asserts that tests passed, but the template has nowhere for the steps a reviewer follows to see that for themselves, so PRs keep inventing the section by hand (#2235 did).
On the fold: collapsing security, performance and breaking changes into one
<details>hides the content that needs the most scrutiny. It also pushes an author to cover those topics twice when they are central rather than incidental, once as a real section and once inside the fold, because leaving the fold empty reads as negligent while burying the content there reads as evasive.Net change: one section added, one fold removed. The template stays short, which is the thing worth protecting about it.
The comment under
## Descriptionnow also mentions###subsections, since a large PR needs its own structure inside that one prose slot.Type of Change
How to test
This PR's own body uses the proposed structure, so it doubles as the preview.
A template only takes effect for PRs opened after it merges, so there is no way to exercise the real thing beforehand. What I did check:
npm run lintpasses, includingscripts/documentation-validator.cjs, which validates.github/PULL_REQUEST_TEMPLATE.md.##sections plus HTML comments, no stray<details>tags left behind.Checklist
npm run lintpassesnpm run test), with tests added where it made senseNo tests: the change is a markdown template with no code path to cover.
Security, performance, or breaking changes
None. Nothing here affects the app, the build, or contributors' existing branches. Open PRs keep whatever body they already have.