New rule: every PR needs a before and an after #232
chaitanyagiri
announced in
Announcements
Replies: 0 comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
Short version: every pull request now has to show a before and an after. Screenshots, or a recording when the thing moves. A PR without them will not merge.
This is live as of today, and it is checked automatically rather than remembered in review.
Why
We are getting far more pull requests than we can review carefully, and that is a good problem that we have been handling badly. The ones that cost the most time are not the difficult ones. They are the ones where the description explains what the author meant to do and nothing in the PR shows what actually changed, so reviewing means checking out the branch and building it just to find out whether it does what it says.
Twenty of those in a week is a week gone. Meanwhile a small, clear, well-evidenced PR sits behind them.
So the rule is now explicit and enforced, rather than a line in
CONTRIBUTING.mdthat most people reasonably never read.What changed
On pull requests. The template now has
### Beforeand### Afterheadings. Drag an image or a video under each. A check calledBefore / after evidenceruns the moment you open the PR, and if either heading is empty it fails, posts a comment saying exactly what is missing, and blocks the merge. Edit the description and it re-runs on save — the loop is seconds, not a review cycle.It reads each heading separately on purpose. Two screenshots dumped under one of them will not pass, because a reviewer has to be able to tell which is which.
"My change has no UI" is not an exemption. It changes what the evidence looks like, not whether you owe it:
Make the two comparable. A light screenshot against a dark one, or different window sizes, means the reviewer ends up diffing your screenshots instead of your change.
There is one way out. The
no-visual-changelabel, for things with genuinely nothing observable — a CI tweak, a typo, a dependency bump. Only maintainers can apply it, so it stays a waiver rather than a self-service opt-out, and every use of it is on the record. Please do not ask for it on something that does have a visible effect; it will cost you more time than the screenshot would have.On issues. Both forms now require a screenshot or recording. Same reasoning: the reports that get fixed first are the ones we can see. If the bug has no visible surface, show the terminal or the log, but show something.
We also fixed something that had been quietly wrong for a while: the bug form asked for your macOS version as a required field, which has been nonsense since the Windows and Linux builds started shipping. It is now an OS dropdown plus a version, and it asks which release of the app you are on.
The bar in general.
CONTRIBUTING.mdhas a new section, What gets a PR closed, which says plainly what we will close rather than negotiate: no evidence, more than one change in a PR, wholesale reformatting, a large rewrite nobody agreed to first, unattributed art, unjustified dependencies.That last one is worth repeating on its own. If you are planning something large, open an issue or a discussion before you write it. We would much rather say no to a paragraph than to your week.
What this is not
It is not aimed at first-time contributors. A small, focused, well-evidenced PR from someone who has never contributed here gets reviewed ahead of a big one from someone who has. The rule exists to make that possible, not to raise a wall.
And one honest limitation, since someone will notice: this cannot stop a PR from being opened. GitHub has no hook that runs before creation, so nothing could. The check fails within seconds of opening instead, and blocks the merge.
Read the details
If you have a case the rule handles badly, say so in this thread. It is new today and we would rather adjust it than have people work around it.
All reactions