This repo has no CONTRIBUTING.md. A contributor has no way to learn what we
expect before they open a pull request, so they find out in review instead.
That happened on #789. The change itself was good. The review still raised
points that came from standards we have never written down, such as which
verbs a commit title may start with. That is not a fair thing to hold
someone to in review.
The file should cover:
- How to run and test a change. Right now there is no test harness for
bin/
and no lint in CI, so say plainly how a contributor should show their work.
- Commit message expectations, and a pointer to
gitmessage, which already
sits in the repo root and which most contributors never open.
- What one pull request should contain.
- What review will hold you to, described below.
Keep it short. A long guide goes stale and nobody reads it.
What review will hold you to
This is the part #789 got wrong, so it deserves its own words.
A review may only ask for a change against a standard this repo publishes.
CONTRIBUTING.md, CODING_STANDARDS.md and gitmessage are that list. If a
reviewer wants a change, they should name the file the rule comes from.
Anything a reviewer wants that is not in one of those files is a preference,
not a defect. Preferences are fine to mention, and they are not grounds to
block a pull request or to send a contributor round again. If a preference
turns out to be worth enforcing, the fix is to add it to one of those files,
where the next contributor can read it before they start.
This binds reviewers whether they are people or agents. An agent reviewing a
branch here reads its own operator's config as well as this repo, and the two
are not the same thing. Only this repo binds a contributor.
A maintainer picking up a follow-up is also often better than a round trip.
Related: #794 covers CODING_STANDARDS.md. This file should point at that
one rather than repeat it.
This repo has no
CONTRIBUTING.md. A contributor has no way to learn what weexpect before they open a pull request, so they find out in review instead.
That happened on #789. The change itself was good. The review still raised
points that came from standards we have never written down, such as which
verbs a commit title may start with. That is not a fair thing to hold
someone to in review.
The file should cover:
bin/and no lint in CI, so say plainly how a contributor should show their work.
gitmessage, which alreadysits in the repo root and which most contributors never open.
Keep it short. A long guide goes stale and nobody reads it.
What review will hold you to
This is the part #789 got wrong, so it deserves its own words.
A review may only ask for a change against a standard this repo publishes.
CONTRIBUTING.md,CODING_STANDARDS.mdandgitmessageare that list. If areviewer wants a change, they should name the file the rule comes from.
Anything a reviewer wants that is not in one of those files is a preference,
not a defect. Preferences are fine to mention, and they are not grounds to
block a pull request or to send a contributor round again. If a preference
turns out to be worth enforcing, the fix is to add it to one of those files,
where the next contributor can read it before they start.
This binds reviewers whether they are people or agents. An agent reviewing a
branch here reads its own operator's config as well as this repo, and the two
are not the same thing. Only this repo binds a contributor.
A maintainer picking up a follow-up is also often better than a round trip.
Related: #794 covers
CODING_STANDARDS.md. This file should point at thatone rather than repeat it.