Boy Scout Rule — Practice Tasks¶
12 hands-on exercises. Every task drops you into a real situation: you opened this file for a small feature or fix, and the code around your change is a little messy. Your job is to leave the campground cleaner than you found it — without turning a 10-line change into a 300-line refactor PR. Each task gives you the actual work item, the messy code you pass through, an instruction, and a full solution that shows the small safe cleanup alongside the feature, what to leave alone (and why), and the reasoning. Languages vary (Go / Java / Python). Difficulty climbs from easy to hard, and at least one task's correct answer is "don't clean this — ticket it."
Table of Contents¶
- Task 1 — Rename a confusing variable next to your change (Python) · Easy
- Task 2 — Delete adjacent dead code (Go) · Easy
- Task 3 — Tidy imports and format only the lines you touched (Python) · Easy
- Task 4 — Extract a tiny helper you're about to call twice (Java) · Easy
- Task 5 — Add a missing test for your change's neighborhood (Go) · Medium
- Task 6 — Split the cleanup into its own commit (Python) · Medium
- Task 7 — Replace a magic number you're already editing (Java) · Medium
- Task 8 — Recognize over-reach and STOP: should you clean this? (Go) · Medium
- Task 9 — Characterization test before a risky tidy (Python) · Hard
- Task 10 — Tidy a confusing conditional inside the block you edit (Java) · Hard
- Task 11 — Resist the rewrite; do the boring incremental cleanup (Go) · Hard
- Task 12 — Boy-Scout audit of a real PR diff (open-ended) · Hard
How to Use¶
- Read the work item first. The cleanup is always secondary to shipping the feature/fix. If the feature breaks because of your cleanup, you failed the rule.
- For each task, before opening the solution, write down two lists: clean now (cheap, safe, in your blast radius) and leave / ticket (out of scope, risky, or large).
- The test for "in scope" is blunt: would a reviewer reading your diff understand why this line changed from the work item alone? If not, it's scope creep — split it out or ticket it.
- Default to keeping cleanups behavior-preserving. Anything that can change runtime behavior needs a test first (Tasks 5, 9).
- One commit should tell one story. When in doubt, see Task 6.
The decision flow every task exercises:
flowchart TD A[Opened file for a small change] --> B{Mess is in the<br/>lines I'm touching?} B -- No --> Z[Leave it. Ticket if it matters] B -- Yes --> C{Cleanup is<br/>behavior-preserving?} C -- No --> D[Add a test first<br/>then tidy] C -- Yes --> E{Cheap and small?<br/>reviewer sees the link} E -- No --> Z E -- Yes --> F[Clean it now,<br/>alongside the change] D --> F F --> G{Cleanup ballooning<br/>past the feature?} G -- Yes --> H[Stop. Split into its<br/>own commit or PR] G -- No --> I[Ship: feature + tidy<br/>in one coherent diff]
Task 1 — Rename a confusing variable next to your change (Python)¶
Difficulty: Easy
Work item: "Add a 5% surcharge for express shipping." The change is one line. But right above it sits a variable named x that you now have to read to make your edit.
def shipping_cost(order):
x = order.weight_kg * 2.5 # base rate per kg
if order.region == "remote":
x = x + 15
return x
# Your one-line feature: express adds 5%.
Instruction: Ship the express surcharge. While you're reading this function to make the change, fix the one thing that actively slowed you down. Decide what not to touch.
Solution
Rename `x` → `cost`. You had to decode `x` just to place your new line correctly, so the rename is squarely in your blast radius and it's purely mechanical (rename-symbol, zero behavior change). **What I cleaned:** `x` → `cost`, and `x = x + 15` → `cost += 15` (the augmented-assignment tidy rides along because that exact line was already being rewritten by the rename). **What I left alone — and why:** - The magic numbers `2.5` and `15` are *not* in my way. Pulling them into named constants is a separate, defensible change, but the reviewer of "add express surcharge" wouldn't expect it. Leave it. (If it nags, ticket it.) - I did **not** rename `order` or restructure the `if`. The rename I did was forced by my reading of the code; these are not. **Reasoning:** The cheapest, safest Boy-Scout cleanup is a local rename of a name you were *forced to understand anyway*. It carries no behavioral risk and makes the very diff you're shipping more readable. That is the rule in its purest form.Task 2 — Delete adjacent dead code (Go)¶
Difficulty: Easy
Work item: "Log the user's plan tier when they hit the paywall." You're editing checkAccess. Right next to it is a helper that nothing calls anymore.
package access
import "log"
func checkAccess(u User, feature string) bool {
if u.IsAdmin {
return true
}
return u.Plan.Allows(feature)
}
// legacyGate was replaced by Plan.Allows in #4821 but never removed.
func legacyGate(u User, feature string) bool {
for _, f := range u.GrantedFeatures {
if f == feature {
return true
}
}
return false
}
Instruction: Add the log line for the paywall. Then handle the dead helper sitting next to your change — but only if you can prove it's safe.
Solution
First, confirm `legacyGate` is truly dead. Run a usage search across the module: If the only hit is the definition itself, delete it. Then add the feature. **What I cleaned:** Removed `legacyGate` entirely (the comment even told us it was orphaned in #4821). **What I left alone — and why:** - I did not go hunting for *other* dead functions in the package. The Boy-Scout boundary is the campground I'm standing in — this file's `checkAccess` neighborhood — not the whole repo. - I kept `u.Plan.Allows` as-is; refactoring it is out of scope. **Reasoning:** Dead code adjacent to your edit is high-value, low-risk to remove *once you've proven it's unreferenced*. Unexported Go funcs make this provable cheaply (grep + staticcheck). Deleting it shrinks the surface the next reader has to understand. The discipline is in *proving* it's dead, not assuming it — and in not turning a paywall log into a repo-wide dead-code sweep.Task 3 — Tidy imports and format only the lines you touched (Python)¶
Difficulty: Easy
Work item: "Parse the created_at field as a timezone-aware datetime." You add two lines. The file's imports are a mess and the indentation is inconsistent across the whole module.
import json,os
from datetime import datetime
import sys
from datetime import timezone # duplicate datetime import line
def load_record(path):
data=json.loads(open(path).read())
record = {
"id": data["id"],
"name": data["name"], # over-indented (pre-existing)
"tags": data.get("tags", []),
}
return record
Instruction: Add timezone-aware parsing of created_at. Clean formatting/imports only where your change lives — do not reformat the whole file.
Solution
import json
import os
import sys
from datetime import datetime, timezone
def load_record(path):
data = json.loads(open(path).read())
record = {
"id": data["id"],
"name": data["name"],
"tags": data.get("tags", []),
"created_at": datetime.fromisoformat(data["created_at"]).astimezone(timezone.utc),
}
return record
Task 4 — Extract a tiny helper you're about to call twice (Java)¶
Difficulty: Easy
Work item: "Trim and lowercase the email on signup too (we already do it on login)." You're adding the same normalization you see on login.
class AuthService {
User login(String email, String password) {
String e = email.trim().toLowerCase(); // normalize
User u = repo.findByEmail(e);
return verify(u, password) ? u : null;
}
User signup(String email, String password) {
// TODO: normalize email here too
if (repo.findByEmail(email) != null) {
throw new IllegalStateException("email exists");
}
return repo.save(new User(email, hash(password)));
}
}
Instruction: Add normalization to signup. Notice you're now writing the same trim().toLowerCase() expression a second time. Make the right small cleanup.
Solution
Extract a one-line helper. You're about to create the second call site, which is exactly when extraction earns its keep (rule of three is generous; two identical normalizations of a *security-relevant* key is enough).class AuthService {
private static String normalizeEmail(String email) {
return email.trim().toLowerCase();
}
User login(String email, String password) {
User u = repo.findByEmail(normalizeEmail(email));
return verify(u, password) ? u : null;
}
User signup(String email, String password) {
String e = normalizeEmail(email);
if (repo.findByEmail(e) != null) {
throw new IllegalStateException("email exists");
}
return repo.save(new User(e, hash(password)));
}
}
Task 5 — Add a missing test for your change's neighborhood (Go)¶
Difficulty: Medium
Work item: "Fix: discount should never make the price negative." There's a one-line clamp to add. The function has no tests at all.
package pricing
func FinalPrice(base, discount int) int {
return base - discount
// bug: if discount > base, this goes negative
}
Instruction: Ship the clamp. Leave the neighborhood safer by adding the test that should have caught this bug — but keep the test scope tight to the function you touched.
Solution
Add the fix and a focused table test covering the behavior you just changed plus the obvious boundaries.package pricing
func FinalPrice(base, discount int) int {
final := base - discount
if final < 0 {
return 0
}
return final
}
package pricing
import "testing"
func TestFinalPrice(t *testing.T) {
cases := []struct {
name string
base, discount int
want int
}{
{"normal discount", 100, 30, 70},
{"no discount", 100, 0, 100},
{"discount equals base", 100, 100, 0},
{"discount exceeds base is clamped", 100, 150, 0}, // the bug this PR fixes
}
for _, c := range cases {
if got := FinalPrice(c.base, c.discount); got != c.want {
t.Errorf("%s: FinalPrice(%d, %d) = %d, want %d",
c.name, c.base, c.discount, got, c.want)
}
}
}
Task 6 — Split the cleanup into its own commit (Python)¶
Difficulty: Medium
Work item: "Add retry (3 attempts) to the webhook sender." While implementing it you also (correctly) rename d → payload, fix a misleading docstring, and pull a magic timeout into a constant. All good cleanups — but now your single commit mixes feature and tidy.
def send(d, url):
"""Sends the data.""" # docstring says nothing useful
resp = requests.post(url, json=d, timeout=5)
resp.raise_for_status()
return resp
Instruction: You've decided all the cleanups are in scope (you touched these lines for the retry). The problem is commit hygiene, not scope. Structure the work so review is easy.
Solution
Make **two commits on the same branch**: one pure-tidy (no behavior change), one feature. Stage them separately. **Commit 1 — `refactor: clarify send() — rename d→payload, real docstring, named timeout`** (behavior-preserving):WEBHOOK_TIMEOUT_SECONDS = 5
def send(payload, url):
"""POST `payload` as JSON to `url`; raise on non-2xx."""
resp = requests.post(url, json=payload, timeout=WEBHOOK_TIMEOUT_SECONDS)
resp.raise_for_status()
return resp
WEBHOOK_TIMEOUT_SECONDS = 5
MAX_ATTEMPTS = 3
def send(payload, url):
"""POST `payload` as JSON to `url`; retry transient failures, raise on final failure."""
last_exc = None
for attempt in range(MAX_ATTEMPTS):
try:
resp = requests.post(url, json=payload, timeout=WEBHOOK_TIMEOUT_SECONDS)
resp.raise_for_status()
return resp
except requests.RequestException as exc:
last_exc = exc
raise last_exc
Task 7 — Replace a magic number you're already editing (Java)¶
Difficulty: Medium
Work item: "Raise the password minimum from 8 to 12 characters." The 8 is a bare literal in a condition you must edit.
class PasswordPolicy {
boolean isValid(String pw) {
if (pw == null) return false;
if (pw.length() < 8) return false;
if (!pw.matches(".*[A-Z].*")) return false;
if (!pw.matches(".*[0-9].*")) return false;
return true;
}
}
Instruction: Change the minimum to 12. Since you're rewriting that exact line, make the constant self-documenting. Decide whether the regex checks are also yours to clean.
Solution
**What I cleaned:** Promoted the bare `8` to a named `MIN_LENGTH = 12` constant. The line was being rewritten anyway, and naming it documents the *intent* (a length policy) rather than a mystery integer — the next person changing the policy will find it immediately. **What I left alone — and why:** - The two `matches(...)` regexes are recompiled on every call (a minor inefficiency) and could be pulled into `static final Pattern` fields. But that's an optimization unrelated to the length bump — touching those lines would make my "raise minimum to 12" diff include changes to the uppercase/digit checks, confusing the reviewer. Ticket it under "precompile password regexes." - I did not add a "special character required" rule. That's a *policy decision*, not a cleanup, and not mine to make in a length-bump ticket. **Reasoning:** Replacing a magic number is the right Boy-Scout move *precisely because you're already editing that line* — the cost is one constant and the benefit is durable readability. The trap is letting "I'm in the password file" justify touching every imperfect line in it. Stay on the line your ticket put you on.Task 8 — Recognize over-reach and STOP: should you clean this? (Go)¶
Difficulty: Medium
Work item: "Add a X-Request-ID header to outgoing API calls." It's a two-line change in doRequest. But the file also contains a 200-line parseResponse function that is genuinely awful: nested switches, duplicated error handling, a 30-line if/else ladder. It offends you. You have time today.
func (c *Client) doRequest(req *http.Request) (*http.Response, error) {
// <-- add X-Request-ID here
return c.http.Do(req)
}
func parseResponse(resp *http.Response) (Result, error) {
// 200 lines of nested switches, duplicated decode/error blocks,
// a giant if/else ladder over status codes... a real mess.
...
}
Instruction: Ship the header. Then make the correct call on parseResponse. The right answer here is the hard one.
Solution
Do the feature. **Do not** refactor `parseResponse`.func (c *Client) doRequest(req *http.Request) (*http.Response, error) {
req.Header.Set("X-Request-ID", uuid.NewString())
return c.http.Do(req)
}
// parseResponse: untouched.
TICKET: Refactor parseResponse (Client) — high complexity, duplicated decode/error handling
parseResponse is ~200 lines: nested switches over status code + content type,
duplicated decode-and-wrap-error blocks, a 30-line if/else ladder. Hard to modify
safely. Proposal: table-driven status handling + a single decode helper. Needs
characterization tests first (no current coverage). Est. 1–2 days. Not coupled to
any feature — schedule on its own.
Task 9 — Characterization test before a risky tidy (Python)¶
Difficulty: Hard
Work item: "Add support for the application/x-ndjson content type." To do it you must edit parse_body, whose logic you find confusing and want to simplify as you go. But it has no tests and the existing branching is subtle — simplifying it blind is risky.
def parse_body(content_type, raw):
if content_type and "json" in content_type:
if raw.strip() == "":
return {}
return json.loads(raw)
elif content_type == "application/x-www-form-urlencoded":
return dict(urllib.parse.parse_qsl(raw))
else:
return {"raw": raw}
Notice the subtlety: "json" in content_type matches application/json, text/json, and application/x-ndjson (because "json" is a substring) — but ndjson is not valid single-object JSON, so the current code would json.loads it and crash. You want to special-case ndjson, and ideally clean the muddy "json" in content_type check.
Instruction: Pin down current behavior with characterization tests before you touch the logic, then make your change and a safe tidy on top of that safety net.
Solution
**Step 1 — Characterization tests** (capture what the code does *today*, including quirks, so a refactor can't silently change it):import pytest
def test_characterize_parse_body():
# JSON
assert parse_body("application/json", '{"a": 1}') == {"a": 1}
# empty JSON body -> {}
assert parse_body("application/json", " ") == {}
# form-encoded
assert parse_body("application/x-www-form-urlencoded", "a=1&b=2") == {"a": "1", "b": "2"}
# unknown type -> wrapped raw
assert parse_body("text/plain", "hello") == {"raw": "hello"}
# None content type -> wrapped raw
assert parse_body(None, "hello") == {"raw": "hello"}
# QUIRK we're about to fix: ndjson currently matches "json" and crashes
with pytest.raises(json.JSONDecodeError):
parse_body("application/x-ndjson", '{"a":1}\n{"a":2}')
def parse_body(content_type, raw):
ct = (content_type or "").split(";")[0].strip() # strip charset params, normalize None
if ct == "application/x-ndjson":
return [json.loads(line) for line in raw.splitlines() if line.strip()]
if ct in ("application/json", "text/json"):
return {} if raw.strip() == "" else json.loads(raw)
if ct == "application/x-www-form-urlencoded":
return dict(urllib.parse.parse_qsl(raw))
return {"raw": raw}
Task 10 — Tidy a confusing conditional inside the block you edit (Java)¶
Difficulty: Hard
Work item: "Also reject requests when the account is suspended." You add one condition to a guard that's already a tangle of nested ifs and negations.
boolean canPlaceOrder(Account acc, Cart cart) {
if (acc != null) {
if (!acc.isClosed()) {
if (cart != null && !cart.isEmpty()) {
if (acc.getBalance() >= cart.total()) {
return true;
} else {
return false;
}
} else {
return false;
}
} else {
return false;
}
} else {
return false;
}
}
Instruction: Add the "suspended account" rejection. The nesting is in the exact block you must edit, so flattening it is in scope — but keep it behavior-preserving and don't gold-plate.
Solution
Add a focused characterization-style test (cheap here — the logic is pure), then flatten to guard clauses and slot in the new rule.boolean canPlaceOrder(Account acc, Cart cart) {
if (acc == null) return false;
if (acc.isClosed()) return false;
if (acc.isSuspended()) return false; // <-- the feature
if (cart == null || cart.isEmpty()) return false;
return acc.getBalance() >= cart.total();
}
@Test void rejectsSuspended() {
assertFalse(canPlaceOrder(suspendedAccount(), nonEmptyCart()));
}
@Test void allowsHealthyOrder() {
assertTrue(canPlaceOrder(fundedOpenAccount(), affordableCart()));
}
@Test void rejectsInsufficientBalance() {
assertFalse(canPlaceOrder(brokeAccount(), expensiveCart()));
}
@Test void rejectsEmptyCart() {
assertFalse(canPlaceOrder(fundedOpenAccount(), emptyCart()));
}
Task 11 — Resist the rewrite; do the boring incremental cleanup (Go)¶
Difficulty: Hard
Work item: "Fix the off-by-one: the last item in the page is being dropped." The pagination function is old and clunky and you're tempted to rewrite it cleanly. There are tests, and they pass.
func Paginate(items []Item, page, size int) []Item {
start := page * size
end := start + size
if end > len(items) {
end = len(items)
}
if start > len(items) { // bug: should clamp start too, but the real bug is below
start = len(items)
}
// off-by-one: callers pass 1-based page, code assumes 0-based
result := []Item{}
for i := start; i < end; i++ {
result = append(result, items[i])
}
return result
}
Instruction: The actual bug is the 1-based vs 0-based page mismatch. Fix it with the smallest correct change. Resist rewriting the whole function — but make a tiny, safe improvement on the line you touch.
Solution
The bug is `start := page * size` assuming 0-based pages while callers pass 1-based. Smallest correct fix: normalize the page once. While I'm editing that line, the manual append loop on the line right after is trivially replaceable with a slice — but I'll weigh it.func Paginate(items []Item, page, size int) []Item {
start := (page - 1) * size // <-- fix: callers pass 1-based pages
if start < 0 {
start = 0
}
end := start + size
if start > len(items) {
start = len(items)
}
if end > len(items) {
end = len(items)
}
return items[start:end] // small safe tidy: loop -> slice, same result
}
Task 12 — Boy-Scout audit of a real PR diff (open-ended)¶
Difficulty: Hard
Work item / scenario: A teammate's PR is titled "Add is_archived flag to notes." You're reviewing it. The diff (below, abbreviated) bundles the feature with a pile of cleanups. Decide, line by line, what's a legitimate Boy-Scout cleanup to keep and what's scope creep that should be split out, ticketed, or reverted.
# notes/service.py
+from typing import Optional
-import os, sys, json # 'os' and 'sys' were unused
+import json
class NoteService:
- def get(self, id):
- n = self.repo.find(id) # 'n' renamed to 'note'
- return n
+ def get(self, id: int) -> Optional["Note"]:
+ note = self.repo.find(id)
+ return note
- def create(self, title, body):
+ def create(self, title, body, is_archived=False): # the feature
note = Note(title=title, body=body)
+ note.is_archived = is_archived
return self.repo.save(note)
+ # NEW: also reformatted the entire 120-line `export_all` method below
+ # NEW: renamed the public method `listNotes` -> `list_notes` (used by 7 callers)
+ # NEW: switched the whole module from `%`-formatting to f-strings (40 lines)
+ # NEW: added a `# TODO: cache this` comment on an unrelated method
Instruction: Produce a review verdict for each change: keep (in-scope tidy) / split (good but belongs in its own commit/PR) / revert + ticket (out of scope). Justify each with the blast-radius and risk tests.
Solution
| Change in the PR | Verdict | Why | |---|---|---| | Remove unused `os`, `sys` imports; split the import line | **Keep** | The import block sits at the top of the file the feature edits, the removal is provably safe (unused), and it's tiny. Classic in-scope tidy. | | Add type hints to `get` (`id: int`, `-> Optional[Note]`) | **Keep (borderline)** | `get` is adjacent and the hints are low-risk documentation. Acceptable if the project uses hints elsewhere. If it'd be the *only* typed method in an untyped file, lean toward **split** to keep the feature diff focused. | | Rename local `n` → `note` in `get` | **Keep** | Local, mechanical, zero behavior change, improves a method right next to the feature. | | Add `is_archived` param + field | **Keep** | This *is* the feature. | | Reformat the entire 120-line `export_all` | **Revert + ticket** | Not in the blast radius (the feature never touches `export_all`), 120 lines of noise that bury the 3-line feature, and pure churn for reviewers. Out of scope. | | Rename public `listNotes` → `list_notes` (7 callers) | **Revert + ticket** (or its own PR) | Public API rename with 7 call sites is a *risky, wide-blast* change — a behavior-relevant refactor masquerading as cleanup. It deserves its own reviewed PR with all call sites visible, not a ride-along on a flag feature. | | Module-wide `%` → f-string (40 lines) | **Split** (own commit/PR) | Defensible cleanup, but unrelated to `is_archived` and large. At minimum its own commit; better its own PR. Bundling it is review sandbagging. | | Add `# TODO: cache this` on an unrelated method | **Revert + ticket** | Drive-by comment on code the PR doesn't touch. If caching matters, file a ticket — a stray TODO is debt that's never paid. | **The verdict in one sentence per anti-pattern:** - *Cleanup avoidance* — none here; the author is the opposite problem. - *Mixed-concern / review sandbagging* — the `export_all` reformat, the f-string sweep, and the public rename all bundle unrelated work into a feature PR. Split them. - *Big-bang over incremental* — the 7-caller public rename is a wide change pretending to be tidy. Isolate it. - *Drive-by on a feature branch without telling reviewers* — the unrelated TODO and the `export_all` reformat. Revert. **The recommendation to the author:** > Keep the import cleanup, the local `n→note` rename, and the `get` hints — they're in the blast radius of the flag change and tiny. Pull the `export_all` reformat, the f-string migration, and the `listNotes` rename into their own PRs (the rename especially — it touches 7 callers and needs its own review). Drop the stray TODO; open a ticket if caching matters. Result: a 3-line feature diff a reviewer can approve in 30 seconds, plus separate, reviewable cleanup PRs. **Reasoning:** The Boy-Scout rule cuts both ways: this PR's author *did* clean things, but smeared three out-of-scope cleanups across a feature, making the whole thing hard to review and risky to merge. The audit applies one consistent test to every line — *blast radius* (did the feature touch this region?) and *risk/size* (is it cheap and behavior-preserving?). Pass both → keep. Fail size/coherence → split. Fail blast radius or risk → revert and ticket.Self-Assessment¶
Rate yourself on each. You've internalized the Boy Scout Rule when you can do all of these by reflex on a real PR.
- I can name my blast radius before cleaning anything — the lines my work item actually touches.
- I clean a confusing local name I was forced to read, without renaming the world (Task 1).
- I delete adjacent dead code only after proving it's unreferenced (Task 2).
- I tidy imports/formatting only inside the hunk I'm changing, never
format-the-whole-file(Task 3). - I extract a helper at the moment duplication appears, and stop short of a full value-object redesign when it's out of scope (Task 4).
- I leave behind the test that pins my fix, scoped to the unit I changed (Task 5).
- I separate behavior-preserving tidy from behavior-changing feature into distinct commits (Task 6).
- I name a magic number I'm already editing, and resist touching unrelated lines in the same file (Task 7).
- I can say "no, ticket it" to a big, risky, out-of-blast-radius mess — and actually file the ticket (Task 8).
- I write characterization tests before tidying risky, untested logic (Task 9).
- I flatten nesting that's in my way without escalating to a pattern the problem doesn't need (Task 10).
- I make the minimal correct fix and resist the rewrite, ticketing the redesign (Task 11).
- I can audit a mixed-concern PR and sort each line into keep / split / revert+ticket (Task 12).
Scoring: 0–4 you're either avoiding cleanup or over-reaching — re-read junior.md. 5–9 solid instincts; tighten your scope discipline. 10–12 you clean the campground and keep the diff honest — the mark of a senior contributor.
Related Topics¶
- junior.md — the Boy Scout Rule from first principles: what "leave it cleaner" means and where its boundary is.
- find-bug.md — spot the bugs that hide in unclean code (and the ones a careless "cleanup" introduces).
- optimize.md — when tidying and performance pull in different directions.
- Chapter README — the positive Clean Code rules this chapter draws on.
- Refactoring section — the catalog of behavior-preserving transformations every safe cleanup uses (Extract Method, Rename, Replace Magic Number, Guard Clauses).
Next: Apply the rule on your next real PR. Before you push, run
git diffand ask the Task 12 question of every changed line: would a reviewer understand why this line changed from the work item alone?