MagicBeanDev/Longshot
MagicBeanDev/Longshot
Watch
75
96
Fork
You've already forked Longshot
 99
Code Issues 230 Pull requests 254 Projects Releases 9 Packages Wiki Activity Actions 823

docs(contributing): a clearer pull request description, and switches set to their recommended value #571

Merged
HumanBean merged 1 commit from HumanBean/Longshot:docs/pr-description into main 2026-11-14 20:16:46 +00:00
Conversation 0 Commits 1 Files changed 11 +1210 -733
HumanBean commented 2026-11-14 20:11:05 +00:00
Owner

Type: docs · Risk: low · Tested on: Linux (game-free checks only) · Needs: any OS, by hand (one check after merge)

Why

Pull request descriptions are hard to review: the reason is often buried, it isn't clear what has and hasn't been tested, and measurements don't say which commits they compare. This rewrites the template so a reviewer sees in a few seconds why the PR exists, whether it helped, and what still has to be tested and where. It also relaxes the switch rule: new switches no longer ship off just because they are new.

No issue covers this (the tracker searched for "template", "description" and "switch").

What changed

The pull request template is rewritten with a status line and clearer sections, and the description checker, forge.py and the contributing docs follow it. Test requests are gone from all of them; work that must be tested elsewhere goes to the Test Farm (@Beanstalk) or is described for someone to do by hand. Switches ship at their recommended value.

  • template: .forgejo/pull_request_template.md. A status line (type, risk, tested on, needs); Why first, with a prompt to search the tracker and link issues; How to reproduce, for fixes only; What changed as a summary, then details; Measurements opening with a verdict, with Before: main @ <hash> / After: this PR @ <hash> columns (variants allowed) or one line saying why nothing was measured; Flags and settings under Measurements (before, after, effect, helps / doesn't help, recommendation); Testing as Tested / Not tested / Must be tested (where, how, expected result, Test Farm or by hand); Risks with Compatibility; Issues (optional); a shorter checklist (Everyone / When it applies).
  • checker: scripts/check_pr_description.py checks the new structure. It requires the status line, How to reproduce when the title's type is fix, a summary before the details, a verdict and commit-named columns (or the one line), complete Flags rows, Must be tested items that name where, how, the expected result and who runs it, and Compatibility. It refuses a bare Fixes #. --title is new; --force still overrides. tests/test_check_pr_description.py and tests/pr_description/good.md are rewritten.
  • forge.py: pr create fills in the status line's type, drops How to reproduce unless it is a fix, and names the columns' commits. pr ready no longer waits on test requests. pr status counts Test Farm and by-hand items. tests/test_forge_pr.py is updated.
  • docs: the switch rule in CONTRIBUTING.md, AGENTS.md and docs/development.md: a change that should have a switch gets one, and it ships at its recommended value (on when that's the recommendation and it's measured). "Costs nothing while off" and the documentation rule stay. CONTRIBUTING.md "Commits and pull requests", AGENTS.md's PR guidance and docs/forge.md describe the new structure. .forgejo/workflows/pr-description.yml changes only its header comment.

Measurements

Verdict: nothing to measure: the template, a script and its tests, and docs; no game code changes.

Flags and settings

None. No switch, menu row, ini key or build flag changes; only the rule for new ones does.

Testing

Tested: on Linux, under nice: tests/test_check_pr_description.py (36 tests), tests/test_forge_pr.py and tests/test_dev_packs.py pass. This description passes the new checker (scripts/check_pr_description.py --file ... --title ...).

Not tested: the whole tests/run_all.py, which wasn't run to keep this PC's CPU free; the changes touch only these scripts, their tests and docs. The pr-description workflow on the forge also hasn't run yet: it runs on this PR.

Must be tested:

  • Any OS, by hand: on the next PR opened after this merges, run python -I scripts/forge.py pr create and check that the pre-filled description follows the new template, expecting the status line, the commit-named columns and only the boxes that apply (by hand: read the pre-filled body before publishing).

Risks

What could break: open PRs written to the old template will fail the new check until their descriptions are rewritten (--force overrides it). pr create pre-fill could be wrong for an unusual branch.

Turning it off: revert the commit.

Compatibility: nothing changes for players. Contributors: descriptions follow the new template, and test requests are no longer accepted as a test plan.

Checklist

Everyone

  • One topic.
  • The title is a Conventional Commit (type(scope): description) of the status line's type.
  • Why gives the reason plainly and links the issues that fit (the tracker searched).
  • Measurements has a verdict, or one line saying why there is nothing to measure.
  • Testing names what was tested, what wasn't, and what must still be tested, where and how.
  • Rebased on current main.
  • python -I -B tests/run_all.py passes (any failure that also fails on main is named). Not run in full: the three affected test files pass (see Testing).
  • No game files, dumps, build output or generated files.
  • third_party/ untouched (a new folder with a PROVENANCE.md instead).
  • The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled.

When it applies

  • AI assistance recorded with Assisted-by: / Co-Authored-By: trailers.
  • Docs updated.
**Type:** docs · **Risk:** low · **Tested on:** Linux (game-free checks only) · **Needs:** any OS, by hand (one check after merge) ## Why Pull request descriptions are hard to review: the reason is often buried, it isn't clear what has and hasn't been tested, and measurements don't say which commits they compare. This rewrites the template so a reviewer sees in a few seconds why the PR exists, whether it helped, and what still has to be tested and where. It also relaxes the switch rule: new switches no longer ship off just because they are new. No issue covers this (the tracker searched for "template", "description" and "switch"). ## What changed The pull request template is rewritten with a status line and clearer sections, and the description checker, `forge.py` and the contributing docs follow it. Test requests are gone from all of them; work that must be tested elsewhere goes to the Test Farm (`@Beanstalk`) or is described for someone to do by hand. Switches ship at their recommended value. - template: `.forgejo/pull_request_template.md`. A status line (type, risk, tested on, needs); Why first, with a prompt to search the tracker and link issues; How to reproduce, for fixes only; What changed as a summary, then details; Measurements opening with a verdict, with `Before: main @ <hash>` / `After: this PR @ <hash>` columns (variants allowed) or one line saying why nothing was measured; Flags and settings under Measurements (before, after, effect, helps / doesn't help, recommendation); Testing as Tested / Not tested / Must be tested (where, how, expected result, Test Farm or by hand); Risks with Compatibility; Issues (optional); a shorter checklist (Everyone / When it applies). - checker: `scripts/check_pr_description.py` checks the new structure. It requires the status line, How to reproduce when the title's type is `fix`, a summary before the details, a verdict and commit-named columns (or the one line), complete Flags rows, Must be tested items that name where, how, the expected result and who runs it, and Compatibility. It refuses a bare `Fixes #`. `--title` is new; `--force` still overrides. `tests/test_check_pr_description.py` and `tests/pr_description/good.md` are rewritten. - forge.py: `pr create` fills in the status line's type, drops How to reproduce unless it is a fix, and names the columns' commits. `pr ready` no longer waits on test requests. `pr status` counts Test Farm and by-hand items. `tests/test_forge_pr.py` is updated. - docs: the switch rule in `CONTRIBUTING.md`, `AGENTS.md` and `docs/development.md`: a change that should have a switch gets one, and it ships at its recommended value (on when that's the recommendation and it's measured). "Costs nothing while off" and the documentation rule stay. `CONTRIBUTING.md` "Commits and pull requests", `AGENTS.md`'s PR guidance and `docs/forge.md` describe the new structure. `.forgejo/workflows/pr-description.yml` changes only its header comment. ## Measurements **Verdict:** nothing to measure: the template, a script and its tests, and docs; no game code changes. ### Flags and settings None. No switch, menu row, ini key or build flag changes; only the rule for new ones does. ## Testing **Tested:** on Linux, under `nice`: `tests/test_check_pr_description.py` (36 tests), `tests/test_forge_pr.py` and `tests/test_dev_packs.py` pass. This description passes the new checker (`scripts/check_pr_description.py --file ... --title ...`). **Not tested:** the whole `tests/run_all.py`, which wasn't run to keep this PC's CPU free; the changes touch only these scripts, their tests and docs. The `pr-description` workflow on the forge also hasn't run yet: it runs on this PR. **Must be tested:** - Any OS, by hand: on the next PR opened after this merges, run `python -I scripts/forge.py pr create` and check that the pre-filled description follows the new template, expecting the status line, the commit-named columns and only the boxes that apply (by hand: read the pre-filled body before publishing). ## Risks **What could break:** open PRs written to the old template will fail the new check until their descriptions are rewritten (`--force` overrides it). `pr create` pre-fill could be wrong for an unusual branch. **Turning it off:** revert the commit. **Compatibility:** nothing changes for players. Contributors: descriptions follow the new template, and test requests are no longer accepted as a test plan. ## Checklist ### Everyone - [x] One topic. - [x] The title is a Conventional Commit (`type(scope): description`) of the status line's type. - [x] Why gives the reason plainly and links the issues that fit (the tracker searched). - [x] Measurements has a verdict, or one line saying why there is nothing to measure. - [x] Testing names what was tested, what wasn't, and what must still be tested, where and how. - [x] Rebased on current `main`. - [x] `python -I -B tests/run_all.py` passes (any failure that also fails on `main` is named). Not run in full: the three affected test files pass (see Testing). - [x] No game files, dumps, build output or generated files. - [x] `third_party/` untouched (a new folder with a `PROVENANCE.md` instead). - [x] The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled. ### When it applies - [x] AI assistance recorded with `Assisted-by:` / `Co-Authored-By:` trailers. - [x] Docs updated.
docs(contributing): a clearer pull request description, and switches set to their recommended value
Some checks failed
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been skipped
Details
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been skipped
Details
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been skipped
Details
Third-party parts / The same as upstream (pull_request) Has been skipped
Details
CI / Conventional Commit title (pull_request) Successful in 4s
Details
CI / Workflow lint (actionlint) (pull_request) Successful in 5s
Details
Third-party parts / As their notes say (pull_request) Successful in 8s
Details
PR description / Description follows the template (pull_request_target) Failing after 4s
Details
beanstalk/approval awaiting maintainer approval
Details
a9f4765c9e 
The pull request template is rewritten so a reviewer sees a PR's state at a
glance: a status line (type, risk, tested on, needs), Why in plain words with
the issues linked, How to reproduce for a fix, What changed as a summary then
details, Measurements with a verdict and Before/After columns named by commit
(or one line when there is nothing to measure) and Flags and settings under
it, Testing with Tested / Not tested / Must be tested (where, how, and the
Test Farm or by hand), Risks with Compatibility, and a shorter checklist
(Everyone / When it applies).

scripts/check_pr_description.py checks the new structure (the status line,
How to reproduce for a fix by the title's type, the summary, the verdict and
commit-named columns, Flags rows, the Must be tested items, Compatibility, a
bare Fixes #) and no longer knows test requests; forge.py pr create fills in
the type, drops How to reproduce for anything but a fix and names the
columns' commits; pr ready and pr status follow. Tests updated.

Switches: a change that should have one still gets one, but it ships at its
recommended value (on by default when measured and recommended), not off for
its own sake: CONTRIBUTING.md, AGENTS.md, docs/development.md.

Assisted-by: Claude Code
Co-Authored-By: Claude Opus 5.5 <[email protected]>
HumanBean changed title from WIP: docs(contributing): a clearer pull request description, and switches set to their recommended value to docs(contributing): a clearer pull request description, and switches set to their recommended value 2026-11-14 20:11:23 +00:00
HumanBean deleted branch docs/pr-description 2026-11-14 20:16:46 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
area/accessibility
Barriers for people with disabilities

  
area/audio
Sound and music

  
area/input
Controllers, keyboard, mouse

  
area/install
Installing, updating, building from your game

  
area/menu
In-game menu and settings

  
area/renderer
Vulkan renderer, shaders, effects

  
area/saves
Save files

  
area/upscaling
DLSS, FSR, frame generation

  
beanstalk: approved @ 1697603
Set by Beanstalk for show; the farm's database is what counts

  
beanstalk: approved @ 2c5a62e
Set by Beanstalk for show; the farm's database is what counts

  
beanstalk: approved @ 4fd82b7
Set by Beanstalk for show; the farm's database is what counts

  
beanstalk: approved @ 64d6d09
Set by Beanstalk for show; the farm's database is what counts

  
beanstalk: approved @ 6f91fae
Set by Beanstalk for show; the farm's database is what counts

  
beanstalk: approved @ 70a78f9
Set by Beanstalk for show; the farm's database is what counts

  
beanstalk: approved @ 8015c9a
Set by Beanstalk for show; the farm's database is what counts

  
beanstalk: approved @ 8d361dc
Set by Beanstalk for show; the farm's database is what counts

  
beanstalk: approved @ 9d8356e
Set by Beanstalk for show; the farm's database is what counts

  
beanstalk: approved @ abf2227
Set by Beanstalk for show; the farm's database is what counts

  
beanstalk: approved @ d82f202
Set by Beanstalk for show; the farm's database is what counts

  
beanstalk: approved @ e5f1e40
Set by Beanstalk for show; the farm's database is what counts

  
beanstalk: approved @ ee8ab5f
Set by Beanstalk for show; the farm's database is what counts

  
ci: approved
A maintainer approved CircleCI for this fork PR (sticky; removed after 14 quiet days)

  
good first issue
A good start for a new contributor

  
gpu/amd
AMD GPUs

  
gpu/igpu
Integrated GPUs

  
gpu/intel
Intel GPUs

  
gpu/nvidia
NVIDIA GPUs

  
help wanted
Extra hands or knowledge wanted

  
kind
bug
 Something doesn't work as it should

  
kind
chore
 Build, CI, dependencies, tidying

  
kind
docs
 Documentation

  
kind
feature
 A new feature or an improvement

  
kind
performance
 Frame rate, frame time, loading, memory

  
kind
question
 A question rather than a change

  
needs measurements
Waiting for before/after measurements

  
platform/linux
Affects Linux (a change for every system carries both Windows and Linux)

  
platform/macos
macOS (Apple silicon or Intel Macs)

  
platform/steam-deck
Steam Deck

  
platform/windows
Affects Windows (a change for every system carries both Windows and Linux)

  
preview
Maintainers: deploy a preview of this PR's website (read its changes first). Removed after the run.

  
priority
critical
 Crashes, data loss, or blocks most players

  
priority
high
 Important: next in line

  
priority
low
 Nice to have

  
regression
Worked in an earlier build, broken in a newer one: highest priority, above other bugs

  
status
blocked
 Waits on something else first

  
status
confirmed
 Reproduced or agreed on

  
status
duplicate
 Already reported or proposed

  
status
invalid
 Not a problem in this project

  
status
needs-info
 Waiting for more information from the reporter

  
status
wontfix
 Will not be worked on

  
testing wanted
Wanted: a test on other hardware (posts a test request in #test-requests)

  
ci: approved
No labels
area/accessibility
area/audio
area/input
area/install
area/menu
area/renderer
area/saves
area/upscaling
beanstalk: approved @ 1697603
beanstalk: approved @ 2c5a62e
beanstalk: approved @ 4fd82b7
beanstalk: approved @ 64d6d09
beanstalk: approved @ 6f91fae
beanstalk: approved @ 70a78f9
beanstalk: approved @ 8015c9a
beanstalk: approved @ 8d361dc
beanstalk: approved @ 9d8356e
beanstalk: approved @ abf2227
beanstalk: approved @ d82f202
beanstalk: approved @ e5f1e40
beanstalk: approved @ ee8ab5f
ci: approved
good first issue
gpu/amd
gpu/igpu
gpu/intel
gpu/nvidia
help wanted
kind
bug
kind
chore
kind
docs
kind
feature
kind
performance
kind
question
needs measurements
platform/linux
platform/macos
platform/steam-deck
platform/windows
preview
priority
critical
priority
high
priority
low
regression
status
blocked
status
confirmed
status
duplicate
status
invalid
status
needs-info
status
wontfix
testing wanted
ci: approved
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
MagicBeanDev/Longshot!571
No description provided.