feat(forge): Flags and settings is a note, never a gate; If applicable keeps only the boxes that apply; the PR template linked up front #76

Merged

HumanBean merged 2 commits from HumanBean/Longshot:pr-flags-note into main 2026-11-13 02:39:00 +00:00

Conversation 2 Commits 2 Files changed 10 +272 -99

HumanBean commented 2026-11-13 01:59:36 +00:00

Owner

• Copy link
• Copy Markdown

Why

The Flags and settings section is a note for reviewers and testers about what a PR adds and what its author recommends, not a reason to hold a PR back; the check failed PRs on it. And the PR template, which every PR uses, was linked only deep inside CONTRIBUTING's steps. And a PR's If applicable list was kept whole, full of unticked or "n/a" boxes that don't apply, which hid the ones that do. This follows up #72, which merged before these changes.

What changed

• check: scripts/check_pr_description.py never fails on Flags and settings. Everything it says about it is a warning (flags_notes, review()): a missing, empty or still-template section, a row missing a cell, an RX_*/HOS_*/LONGSHOT_* name the diff adds that the note doesn't list, a vk_menu.cpp change while it says None. Flags left REQUIRED. The same goes for pr ready and the workflow, which run the same check; pr status prints them as note: lines.
• template: the Flags table is leaner, Flag | What it does | Default now | Recommendation | Helps on | Doesn't help on | Untested on, with "Recommendation" (on by default / off by default / on after more testing, with a short reason) in place of "Goal: on by default?". Its comment says it is a note for reviewers and testers: what the PR adds and what you recommend. "None" stays for PRs without flags. The If applicable box reads "Flags noted, and the switch reference updated (docs/development.md, the README)".
• CONTRIBUTING.md: "Commits and pull requests" opens with "Every pull request uses the pull request template: …"; the Measurements and Testing paragraphs link it too, and Flags and settings is described as a note with a recommendation. AGENTS.md's PR notes link the template; docs/forge.md's pr ready says Flags only warns.
• If applicable (second commit): a PR keeps only the boxes that apply, each ticked. The check fails a box kept but unticked, warns on "n/a" (delete it instead), and hints (a warning) at a box the diff suggests but that isn't there (suggested_boxes, HINTS: gpu/vk/vk_api.h, a shader, a new switch, host code, a source file tests read, the docs). For everyone stays whole. pr create keeps only the suggested boxes (none: the heading and a comment pointing at the template, keep_applicable); pr status shows "If applicable: N kept, N ticked" and the hints. The template keeps the full list as the menu, with the comment "Keep only the boxes that apply to this PR, and tick them; delete the rest. Keep the whole For everyone list."; the workflow's checkout adds tests/ for the hint. CONTRIBUTING, AGENTS.md and docs/forge.md say so.
• tests: tests/test_check_pr_description.py (If applicable trimmed, unticked, n/a and the hints; Flags as warnings, never failures; the new columns), tests/test_forge_pr.py (pr ready goes ahead with a Flags warning, pr status shows it as a note; pr create keeps the suggested boxes), tests/pr_description/good.md with the new table and only the boxes that apply to it.

Flags and settings

None

Measurements

main: 77f605a2 (2026-11-13)
this PR: pr-flags-note @ 3232d7f4
Hardware: Ryzen desktop, CachyOS (Linux 7.3), Python 3.13
Scene: no game run: the game-free checks only
Method: LONGSHOT_VULKAN_HEADERS=/usr/include python -I -B tests/run_all.py, the whole run, two interleaved runs each (main, this PR, main, this PR); the two description tests by themselves on each side
How to reproduce: jj new main@forge or this branch, then the command above; python -I -B tests/test_check_pr_description.py; python -I -B tests/test_forge_pr.py

	main @ 77f605a2	this PR @ 3232d7f4	Δ
tests/run_all.py: passed (run 1 / run 2)	490 / 490	490 / 490	0 (no new test file)
tests/run_all.py: failed	2 / 2	2 / 2	0 (the same two)
tests/run_all.py: skipped	36 / 36	36 / 36	0
tests/run_all.py: time (run 1 / run 2)	87 / 91 s	93 / 86 s	about the same
tests/test_check_pr_description.py	18 tests pass	22 tests pass	+4
tests/test_forge_pr.py	19 tests pass	20 tests pass	+1

main @ 77f605a2:
490 passed, 2 failed, 36 skipped in 87 s (linux, 24 at once)
490 passed, 2 failed, 36 skipped in 91 s (linux, 24 at once)
this PR @ 3232d7f4:
490 passed, 2 failed, 36 skipped in 93 s (linux, 24 at once)
490 passed, 2 failed, 36 skipped in 86 s (linux, 24 at once)
both, each run:
FAIL tests/test_bake_portability.py
FAIL tests/test_builder_log_read_only.py

The two failures are the known ones here, on main as well (test_bake_portability: no zstandard; test_builder_log_read_only: no libtk).

Testing

Tested: on the Linux PC above: the whole game-free suite, twice on each side, and the two description tests; on git.magicbean.dev, this PR itself: opened with forge.py pr create, filled in with pr update, then pr ready and pr status.

Not tested: Windows: no Windows PC here; the change is text handling in standard-library Python, with no path or OS code touched. The PR description workflow on a runner (#13 moved the workflows, but none has run this one yet).

Needs other testers: None: nothing here depends on the hardware or the OS.

Screenshots

Nothing on screen.

Risk and rollback

A PR that adds a switch without noting it now goes ready with a warning instead of being refused, so reviewers rely on the note and the warning. A box kept but unticked now fails where it used to pass, so a PR copied from the old template must delete the boxes that don't apply. Reverting the commits brings back the Flags gate, the old columns and the whole If applicable list.

Reviewer notes

Start with the Flags and checklist parts of review() and suggested_boxes in scripts/check_pr_description.py. Descriptions written with the old Flags columns still pass; their rows are read by the header they have.

Issues

See #72

Checklist

For everyone

• One topic.
• The title is a Conventional Commit (type(scope): description).
• Rebased on current main.
• python -I -B tests/run_all.py run and passing (any failure that also fails on main is named above).
• Tested in the game, or what wasn't is named under Testing.
• Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests).
• Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported.
• 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.

If applicable

• A check under tests/ that reads changed source updated in this PR, not weakened.
• Docs updated.

## Why The Flags and settings section is a note for reviewers and testers about what a PR adds and what its author recommends, not a reason to hold a PR back; the check failed PRs on it. And the PR template, which every PR uses, was linked only deep inside CONTRIBUTING's steps. And a PR's If applicable list was kept whole, full of unticked or "n/a" boxes that don't apply, which hid the ones that do. This follows up #72, which merged before these changes. ## What changed - check: `scripts/check_pr_description.py` never fails on Flags and settings. Everything it says about it is a warning (`flags_notes`, `review()`): a missing, empty or still-template section, a row missing a cell, an `RX_*`/`HOS_*`/`LONGSHOT_*` name the diff adds that the note doesn't list, a `vk_menu.cpp` change while it says None. Flags left `REQUIRED`. The same goes for `pr ready` and the workflow, which run the same check; `pr status` prints them as `note:` lines. - template: the Flags table is leaner, Flag | What it does | Default now | Recommendation | Helps on | Doesn't help on | Untested on, with "Recommendation" (on by default / off by default / on after more testing, with a short reason) in place of "Goal: on by default?". Its comment says it is a note for reviewers and testers: what the PR adds and what you recommend. "None" stays for PRs without flags. The If applicable box reads "Flags noted, and the switch reference updated (docs/development.md, the README)". - CONTRIBUTING.md: "Commits and pull requests" opens with "Every pull request uses the [pull request template](.forgejo/pull_request_template.md): …"; the Measurements and Testing paragraphs link it too, and Flags and settings is described as a note with a recommendation. AGENTS.md's PR notes link the template; `docs/forge.md`'s `pr ready` says Flags only warns. - If applicable (second commit): a PR keeps only the boxes that apply, each ticked. The check fails a box kept but unticked, warns on "n/a" (delete it instead), and hints (a warning) at a box the diff suggests but that isn't there (`suggested_boxes`, `HINTS`: `gpu/vk/vk_api.h`, a shader, a new switch, host code, a source file tests read, the docs). For everyone stays whole. `pr create` keeps only the suggested boxes (none: the heading and a comment pointing at the template, `keep_applicable`); `pr status` shows "If applicable: N kept, N ticked" and the hints. The template keeps the full list as the menu, with the comment "Keep only the boxes that apply to this PR, and tick them; delete the rest. Keep the whole For everyone list."; the workflow's checkout adds `tests/` for the hint. CONTRIBUTING, AGENTS.md and `docs/forge.md` say so. - tests: `tests/test_check_pr_description.py` (If applicable trimmed, unticked, n/a and the hints; Flags as warnings, never failures; the new columns), `tests/test_forge_pr.py` (`pr ready` goes ahead with a Flags warning, `pr status` shows it as a note; `pr create` keeps the suggested boxes), `tests/pr_description/good.md` with the new table and only the boxes that apply to it. ## Flags and settings None ## Measurements main: 77f605a2 (2026-11-13) this PR: pr-flags-note @ 3232d7f4 Hardware: Ryzen desktop, CachyOS (Linux 7.3), Python 3.13 Scene: no game run: the game-free checks only Method: `LONGSHOT_VULKAN_HEADERS=/usr/include python -I -B tests/run_all.py`, the whole run, two interleaved runs each (main, this PR, main, this PR); the two description tests by themselves on each side How to reproduce: `jj new main@forge` or this branch, then the command above; `python -I -B tests/test_check_pr_description.py`; `python -I -B tests/test_forge_pr.py` | | main @ 77f605a2 | this PR @ 3232d7f4 | Δ | |---|---|---|---| | tests/run_all.py: passed (run 1 / run 2) | 490 / 490 | 490 / 490 | 0 (no new test file) | | tests/run_all.py: failed | 2 / 2 | 2 / 2 | 0 (the same two) | | tests/run_all.py: skipped | 36 / 36 | 36 / 36 | 0 | | tests/run_all.py: time (run 1 / run 2) | 87 / 91 s | 93 / 86 s | about the same | | tests/test_check_pr_description.py | 18 tests pass | 22 tests pass | +4 | | tests/test_forge_pr.py | 19 tests pass | 20 tests pass | +1 | ``` main @ 77f605a2: 490 passed, 2 failed, 36 skipped in 87 s (linux, 24 at once) 490 passed, 2 failed, 36 skipped in 91 s (linux, 24 at once) this PR @ 3232d7f4: 490 passed, 2 failed, 36 skipped in 93 s (linux, 24 at once) 490 passed, 2 failed, 36 skipped in 86 s (linux, 24 at once) both, each run: FAIL tests/test_bake_portability.py FAIL tests/test_builder_log_read_only.py ``` The two failures are the known ones here, on main as well (`test_bake_portability`: no zstandard; `test_builder_log_read_only`: no libtk). ## Testing **Tested:** on the Linux PC above: the whole game-free suite, twice on each side, and the two description tests; on git.magicbean.dev, this PR itself: opened with `forge.py pr create`, filled in with `pr update`, then `pr ready` and `pr status`. **Not tested:** Windows: no Windows PC here; the change is text handling in standard-library Python, with no path or OS code touched. The `PR description` workflow on a runner (#13 moved the workflows, but none has run this one yet). **Needs other testers:** None: nothing here depends on the hardware or the OS. ## Screenshots Nothing on screen. ## Risk and rollback A PR that adds a switch without noting it now goes ready with a warning instead of being refused, so reviewers rely on the note and the warning. A box kept but unticked now fails where it used to pass, so a PR copied from the old template must delete the boxes that don't apply. Reverting the commits brings back the Flags gate, the old columns and the whole If applicable list. ## Reviewer notes Start with the Flags and checklist parts of `review()` and `suggested_boxes` in `scripts/check_pr_description.py`. Descriptions written with the old Flags columns still pass; their rows are read by the header they have. ## Issues See #72 ## Checklist ### For everyone - [x] One topic. - [x] The title is a Conventional Commit (`type(scope): description`). - [x] Rebased on current `main`. - [x] `python -I -B tests/run_all.py` run and passing (any failure that also fails on `main` is named above). - [x] Tested in the game, or what wasn't is named under Testing. - [x] Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests). - [x] Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported. - [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. ### If applicable - [x] A check under `tests/` that reads changed source updated in this PR, not weakened. - [x] Docs updated.

HumanBean added 1 commit 2026-11-13 01:59:36 +00:00

feat(forge): Flags and settings is a note with a recommendation, never a gate; the PR template linked up front ... f5b8a3167f

The description check, pr ready and the workflow only warn on the Flags and settings section (missing, empty, a row
missing a cell, a switch the diff adds that it doesn't list, a menu change with None); pr status shows them as notes.
The table is leaner: Flag | What it does | Default now | Recommendation | Helps on | Doesn't help on | Untested on, the
recommendation on by default, off by default or on after more testing, with why. CONTRIBUTING.md opens its pull
request section with the template, and links it from Measurements and Testing; AGENTS.md links it too.

HumanBean commented 2026-11-13 02:00:07 +00:00

Author

Owner

• Copy link
• Copy Markdown

Filled in the description. The Flags and settings section is now a note with a recommendation, never a gate (the check, pr ready and the workflow only warn; pr status shows notes), the table is leaner with a Recommendation column, and CONTRIBUTING and AGENTS.md link the PR template up front. Tests on main @ ef8b784f and this PR @ f5b8a316, two interleaved runs each: 491 passed and the same 2 known failures on both. This follows up #72, which merged before these changes.

Filled in the description. The Flags and settings section is now a note with a recommendation, never a gate (the check, pr ready and the workflow only warn; pr status shows notes), the table is leaner with a Recommendation column, and CONTRIBUTING and AGENTS.md link the PR template up front. Tests on main @ ef8b784f and this PR @ f5b8a316, two interleaved runs each: 491 passed and the same 2 known failures on both. This follows up #72, which merged before these changes.

HumanBean changed title from WIP: feat(forge): Flags and settings is a note with a recommendation, never a gate; the PR template linked up front to feat(forge): Flags and settings is a note with a recommendation, never a gate; the PR template linked up front 2026-11-13 02:00:07 +00:00

HumanBean force-pushed pr-flags-note from f5b8a3167f to 3232d7f494 2026-11-13 02:17:10 +00:00 Compare

HumanBean commented 2026-11-13 02:17:36 +00:00

Author

Owner

• Copy link
• Copy Markdown

Pushed 3232d7f4, rebased onto main @ 77f605a2 (#13): If applicable now keeps only the boxes that apply, each ticked. The check fails a box kept but unticked, warns on n/a (delete it instead) and hints at a box the diff suggests but that isn't there (vk_api.h, a shader, a new switch, host code, a source file tests read, the docs); For everyone stays whole. pr create keeps only the suggested boxes, and pr status shows 'N kept, N ticked' with the hints. The description follows the new rule (two boxes kept), with the tests measured again: main @ 77f605a2 and this PR @ 3232d7f4, two interleaved runs each, 490 passed and the same 2 known failures on both.

Pushed 3232d7f4, rebased onto main @ 77f605a2 (#13): If applicable now keeps only the boxes that apply, each ticked. The check fails a box kept but unticked, warns on n/a (delete it instead) and hints at a box the diff suggests but that isn't there (vk_api.h, a shader, a new switch, host code, a source file tests read, the docs); For everyone stays whole. pr create keeps only the suggested boxes, and pr status shows 'N kept, N ticked' with the hints. The description follows the new rule (two boxes kept), with the tests measured again: main @ 77f605a2 and this PR @ 3232d7f4, two interleaved runs each, 490 passed and the same 2 known failures on both.

HumanBean changed title from feat(forge): Flags and settings is a note with a recommendation, never a gate; the PR template linked up front to feat(forge): Flags and settings is a note, never a gate; If applicable keeps only the boxes that apply; the PR template linked up front 2026-11-13 02:17:49 +00:00

HumanBean merged commit 5eaaf7fb9c into main 2026-11-13 02:39:00 +00:00

HumanBean deleted branch pr-flags-note 2026-11-13 02:39:01 +00:00

HumanBean referenced this pull request from a commit 2026-11-13 02:39:01 +00:00

feat(forge): Flags and settings is a note, never a gate; If applicable keeps only the boxes that apply; the PR template linked up front (#76)

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

accessibility

Barrier affecting people with disabilities

  

autorelease: pending

  

autorelease: tagged

  

bug

Something isn't working

  

dependencies

Pull requests that update a dependency file

  

documentation

Improvements or additions to documentation

  

duplicate

This issue or pull request already exists

  

enhancement

New feature or request

  

github_actions

Pull requests that update GitHub Actions code

  

good first issue

Good for newcomers

  

help wanted

Extra attention is needed

  

invalid

This doesn't seem right

  

needs measurements

Waiting for before/after measurements

  

question

Further information is requested

  

testing wanted

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

  

wontfix

This will not be worked on

No labels

accessibility

autorelease: pending

autorelease: tagged

bug

dependencies

documentation

duplicate

enhancement

github_actions

good first issue

help wanted

invalid

needs measurements

question

testing wanted

wontfix

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

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!76

No description provided.