fleetd #748: five code-quality rules enter the repo
Two architects reviewed the codebase independently on separate backends and both returned MIXED, not "a mess": 0 public mutable fields, 12 extends of which 8 are exceptions, 120 records, and 2 files over 1000 code lines rather than the 9 a total-line count suggests. Both rejected a Clean Code section, a SOLID list, a pattern catalogue, class and method length limits, and a coverage gate as text that would change no behaviour. The comment rule they were briefed against was never in this repo. It lives in the operator's personal global config, so it was not in git, never reviewed, and not versioned with the code. Its "goes in the ADR" line was unfollowable because this project has no ADR, which sent load-bearing knowledge to a destination that does not exist. Rule 2 redirects it to docs/<subject>.md, which exists. Rule 4 covers what neither architect ranked first and both described: a testing problem relieved by reshaping production code. 54 static factories on Fleetd, 7 volatile race hooks in MessageService, 8 tests asserting on main source as text, and a 452-line composition root across 8 tickets. Rule 4's judgement half is marked as having no mechanism. A cap stops a count growing; no test distinguishes a good decomposition from a bad one. Verified: canonical block still byte-identical with wiki/7-Use-Cases.md.
This commit is contained in:
@@ -502,3 +502,33 @@ to replace them.
|
||||
|
||||
Prefer the unnamed lambda parameter `_` for required-but-unused params; a non-public
|
||||
`static void main(String[])` is valid (JEP 512) and boots via `java -jar`.
|
||||
|
||||
## Code quality — five rules, and what each already cost (enforced)
|
||||
|
||||
Measured at `7f0c4a8`: 124 main files, 36,278 lines, of which **17,850 are code** — 44% is comment,
|
||||
and only **two** files exceed 1000 *code* lines. Encapsulation and inheritance are already sound (0
|
||||
public mutable fields; 12 `extends`, 8 of them exceptions; 120 records). So there is **no Clean Code
|
||||
section, no SOLID list and no pattern catalogue** here: two architect reviews rejected those
|
||||
independently as text that would change no behaviour. These five rules are the whole standard.
|
||||
|
||||
1. **A comment states the current contract or a current maintainer constraint — nothing else.** No
|
||||
tickets, history, dates, measurements or review rationale; those go in the commit message or the
|
||||
MR description. Source code only — this rule never applies to Markdown.
|
||||
2. **A javadoc block stops at 30 lines.** Longer means it is a design argument, so it moves to
|
||||
`docs/<subject>.md` and is linked in one line. The longest here is 235 lines
|
||||
(`config/ConfigRef.java`) and the knowledge in it is load-bearing: **move it, never delete it.**
|
||||
This project has **no ADR** — subject pages under `docs/` are the destination.
|
||||
3. **A comment in main source never names a test class.** There are 44 such names in 76 places and
|
||||
**2 are already dead**, because a name inside `{@code}` is invisible to the compiler and rots in
|
||||
silence. Say what the code guarantees; the test is found by looking.
|
||||
4. **Never relieve a testing problem by reshaping production code.** `Fleetd` carries 54 static
|
||||
factories, `MessageService` carries 7 `volatile` race hooks, and 8 tests assert on main source as
|
||||
*text*. Make the part injectable instead. `FleetdAssembly.assembleAndStart` is 452 lines and may
|
||||
not grow; no new source-text test may be added.
|
||||
5. **No new package cycle, and no widening of a recorded one.** Five pairs are frozen as an exact
|
||||
edge baseline in `PackageCyclesTest` — four of them involve `msg`.
|
||||
|
||||
Rules 1, 2, 3 and 5 have build checks, and Gitea CI runs them on every PR, so they bind members too.
|
||||
**Rule 4's judgement half has no mechanism**: a cap stops a count growing, but no test tells a good
|
||||
decomposition from a bad one. That half is a review obligation, and saying so is deliberate — a rule
|
||||
dressed as a gate it does not have is worse than an honest review item.
|
||||
|
||||
Reference in New Issue
Block a user