From 4628458ab03e48a7109f985f409e81ae7dd63c8a Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Sun, 5 Apr 2026 08:39:50 +0200 Subject: [PATCH] chore(process): add zero-warnings policy to pr-push and pr-review - pr-push step 1b: lint check before pushing (gdlint, clippy, ruff) - pr-review step 0c: check warning count before spawning reviewers - Added .gdlintrc with max-line-length: 120 - Created ticket #783 for Sprint 31: clean up existing 253 warnings - Updated #780 with edge visibility rule (show edges only for selected system) Co-Authored-By: Claude Opus 4.6 (1M context) --- .claude/skills/pr-push/SKILL.md | 30 +++++++++++++++++- .claude/skills/pr-review/SKILL.md | 13 ++++++++ .gdlintrc | 47 ++++++++++++++++++++++++++++ docs/backups/settledreach.db.backup | Bin 962560 -> 962560 bytes 4 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 .gdlintrc diff --git a/.claude/skills/pr-push/SKILL.md b/.claude/skills/pr-push/SKILL.md index b770405bb..b488643e9 100644 --- a/.claude/skills/pr-push/SKILL.md +++ b/.claude/skills/pr-push/SKILL.md @@ -34,7 +34,35 @@ git branch --show-current If on `main`, stop: "You're on main. Switch to a team branch first." -### 1b. Runtime smoke test (MANDATORY) +### 1b. Zero warnings policy (MANDATORY) + +Before pushing, verify the branch has **zero lint warnings**. Any warning +must be either fixed or suppressed with a commented justification. + +**For client/visual branches:** +```bash +gdlint client/scripts/ client/ui/ 2>&1 +``` + +If warnings remain, fix them before pushing. For warnings that cannot be +fixed (e.g. intentional long lines in data literals), add a `# gdlint: +ignore` comment with a reason. + +**For server branches:** +```bash +cargo clippy -- -D warnings 2>&1 +``` + +**For CI/tooling branches:** +```bash +ruff check tooling/ 2>&1 +``` + +The goal is zero warnings in the pre-push output. Advisory warnings that +the pre-push hook reports as "(advisory, not blocking)" should still be +zero — they are advisory only because we haven't enforced them yet. + +### 1c. Runtime smoke test (MANDATORY) Before pushing, verify the game actually runs. This is non-negotiable — Sprint 28 proved that code review without runtime testing misses critical diff --git a/.claude/skills/pr-review/SKILL.md b/.claude/skills/pr-review/SKILL.md index ab34e5481..fc852dbbe 100644 --- a/.claude/skills/pr-review/SKILL.md +++ b/.claude/skills/pr-review/SKILL.md @@ -47,6 +47,19 @@ godot --headless --path client --quit 2>&1 | grep -i "SCRIPT ERROR" If script errors appear in the branch diff files, flag them immediately before spawning reviewers — no point reviewing code that doesn't parse. +### 0c. Zero warnings check + +The project enforces a **zero warnings policy**. Before spawning reviewers, +check if the branch introduces lint warnings: + +- **client/visual:** `gdlint client/scripts/ client/ui/` should report 0 issues +- **server:** `cargo clippy -- -D warnings` should be clean +- **ci/tooling:** `ruff check tooling/` should be clean + +If warnings exist, note the count in the review output. Reviewers should +flag any **new** warnings introduced by the branch as `warning` severity. +Pre-existing warnings are not PR blockers but should be tracked for cleanup. + ### 1. Determine the branch to review If the user provided a branch name as argument, use it. Otherwise list open diff --git a/.gdlintrc b/.gdlintrc new file mode 100644 index 000000000..b9e3f93df --- /dev/null +++ b/.gdlintrc @@ -0,0 +1,47 @@ +class-definitions-order: +- tools +- classnames +- extends +- docstrings +- signals +- enums +- consts +- staticvars +- exports +- pubvars +- prvvars +- onreadypubvars +- onreadyprvvars +- others +class-load-variable-name: (([A-Z][a-z0-9]*)+|_?[a-z][a-z0-9]*(_[a-z0-9]+)*) +class-name: ([A-Z][a-z0-9]*)+ +class-variable-name: _?[a-z][a-z0-9]*(_[a-z0-9]+)* +comparison-with-itself: null +constant-name: _?[A-Z][A-Z0-9]*(_[A-Z0-9]+)* +disable: [] +duplicated-load: null +enum-element-name: '[A-Z][A-Z0-9]*(_[A-Z0-9]+)*' +enum-name: ([A-Z][a-z0-9]*)+ +excluded_directories: !!set + .git: null +expression-not-assigned: null +function-argument-name: _?[a-z][a-z0-9]*(_[a-z0-9]+)* +function-arguments-number: 10 +function-name: (_on_([A-Z][a-z0-9]*)+(_[a-z0-9]+)*|_?[a-z][a-z0-9]*(_[a-z0-9]+)*) +function-preload-variable-name: ([A-Z][a-z0-9]*)+ +function-variable-name: '[a-z][a-z0-9]*(_[a-z0-9]+)*' +load-constant-name: (([A-Z][a-z0-9]*)+|_?[A-Z][A-Z0-9]*(_[A-Z0-9]+)*) +loop-variable-name: _?[a-z][a-z0-9]*(_[a-z0-9]+)* +max-file-lines: 1000 +max-line-length: 120 +max-public-methods: 20 +max-returns: 6 +mixed-tabs-and-spaces: null +no-elif-return: null +no-else-return: null +signal-name: '[a-z][a-z0-9]*(_[a-z0-9]+)*' +sub-class-name: _?([A-Z][a-z0-9]*)+ +tab-characters: 1 +trailing-whitespace: null +unnecessary-pass: null +unused-argument: null diff --git a/docs/backups/settledreach.db.backup b/docs/backups/settledreach.db.backup index d79571e51358f161aac966e94414b79020438a99..7e0fa6ff5a261ee663a2c4f7a7d35db3c3e04c11 100644 GIT binary patch delta 1363 zcmZvbU1%It6vyYz$F95C=A#&E;>!^q+(7mtyV$R#u%{Jo0+@2)6C8c z_s(P!AG)dHmLf>ez)A&CEYb%lf*)6qg8CpW2rY=Uk7*U3L=-`M5k0e;=!*}-?w+~l zfByG({=2-_;AG^XQ*?T8_V`Pxp~C*z$bL&=llaC?Pa9E9$7XJz4al26=tooJHEH~+ZxJh&y#4%7uU`_ZnDjp;~t$GTxKLtJ%08YMrGfE_AOPO{#FZ!5r`%>bi_W zx*B3Q7(6z0xU}!qbDgd3u)609L=-d;1c)Hoa1>~ZVK&@uGLp$k-Chw9R|?C|0S_G( z1uy7bl){EBLiCFnt1<*=MjCEjga3Br{*;I~;N-ZBi6Si^Im%XKUUGwTcUsUU*oiT~~AX5puwZIs14 zm7ke1jvdq;$5UWzh)N%D3g+t+ojqcuf*BHTH~+GwX}V`B56ThGvJtY2gOs?~miowCp7v^LurC1