docs: remove the whole-project review agenda and its references
REVIEW.md and the /review-project command are gone; other docs no longer point at them. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -1,26 +0,0 @@
|
||||
---
|
||||
description: Review the project as a whole against the agenda in docs/REVIEW.md
|
||||
---
|
||||
|
||||
Perform a whole-project review of GoSentry.
|
||||
|
||||
Read [docs/REVIEW.md](../../docs/REVIEW.md) first — it is the agenda, and its
|
||||
nine sections are the areas to cover. Read [docs/STANDARDS.md](../../docs/STANDARDS.md)
|
||||
and [docs/ARCHITECTURE.md](../../docs/ARCHITECTURE.md) for the rules and
|
||||
contracts the code is checked against.
|
||||
|
||||
$ARGUMENTS narrows the review when given — a package path, a file, or the name
|
||||
of an agenda section. With no arguments, sweep the whole `src/` tree.
|
||||
|
||||
Rules for the report:
|
||||
|
||||
- Anything listed under "Intentional behavior" in STANDARDS.md is not a finding.
|
||||
If you believe such an entry is now wrong, say so explicitly as a challenge to
|
||||
the decision rather than reporting it as a bug.
|
||||
- Verify before reporting. Read the surrounding code and, where cheap, confirm
|
||||
the behavior with a test rather than reasoning about it alone.
|
||||
- Group findings by agenda section, most severe first, each with the file and
|
||||
line and what would actually go wrong.
|
||||
- Report honestly that a section is clean rather than inventing something for it.
|
||||
- Do not fix anything during the review. Report first; apply fixes only when
|
||||
asked, following "What happens to the findings" in REVIEW.md.
|
||||
@@ -13,12 +13,6 @@ application service, scheduler, storage, and command runner in one binary.
|
||||
- [docs/TESTS.md](docs/TESTS.md) — test layout and conventions.
|
||||
- [docs/ROADMAP.md](docs/ROADMAP.md) — deliberately out of scope.
|
||||
|
||||
## Reviewing the project
|
||||
|
||||
When the user asks for a review of the project (rather than of a specific
|
||||
diff), follow [docs/REVIEW.md](docs/REVIEW.md) — it is the agenda, and the
|
||||
`/review-project` command runs the same thing. Do not improvise a checklist.
|
||||
|
||||
## Key rules (full list in STANDARDS.md)
|
||||
|
||||
- `src/app.Service` is the sole owner of job and runtime state; the UI reads it
|
||||
|
||||
@@ -51,7 +51,6 @@ GoSentry is built and tested on **Windows** and **Linux**:
|
||||
- [Roadmap](docs/ROADMAP.md) — planned work larger than a single bug fix
|
||||
- [Architecture](docs/ARCHITECTURE.md) — component interaction model
|
||||
- [Standards](docs/STANDARDS.md) — quality rules and intentional behavior
|
||||
- [Review](docs/REVIEW.md) — what a whole-project review looks at
|
||||
- [Development](docs/DEVELOPMENT.md) — build instructions, project layout, dependencies
|
||||
- [Tests](docs/TESTS.md) — test suite layout and how to run it
|
||||
- [Performance](docs/PERFORMANCE.md) — measured performance findings
|
||||
|
||||
@@ -1,9 +1,9 @@
|
||||
# Whole-project review — action plan
|
||||
|
||||
Working document for the findings of the 2026-08-05 whole-project review, run
|
||||
against the agenda in [REVIEW.md](REVIEW.md). It is not part of the permanent
|
||||
doc set: delete it once every item below is either done or moved to
|
||||
[ROADMAP.md](ROADMAP.md), the way `TEST_REVIEW_PLAN.md` was retired.
|
||||
Working document for the findings of the 2026-08-05 whole-project review. It is
|
||||
not part of the permanent doc set: delete it once every item below is either
|
||||
done or moved to [ROADMAP.md](ROADMAP.md), the way `TEST_REVIEW_PLAN.md` was
|
||||
retired.
|
||||
|
||||
The rules the findings are judged against live in [STANDARDS.md](STANDARDS.md)
|
||||
and [ARCHITECTURE.md](ARCHITECTURE.md). Anything listed under "Intentional
|
||||
@@ -41,8 +41,8 @@ The findings below are therefore mostly about **the paths that only show up
|
||||
after the app has been running for a while** (§3.1, §6.3), **durability of the
|
||||
JSON files** (§6.2), and **one confirmed Windows quoting bug** (§6.1).
|
||||
|
||||
Severity is used as REVIEW.md uses it: *medium* means it gets a regression test
|
||||
with the fix.
|
||||
Severity follows the whole-project review convention: *medium* means it gets a
|
||||
regression test with the fix.
|
||||
|
||||
---
|
||||
|
||||
@@ -717,7 +717,7 @@ entries. These four are not.
|
||||
## Suggested order
|
||||
|
||||
Grouped so that each commit is independently reviewable and each medium finding
|
||||
lands with its regression test, per REVIEW.md's "What happens to the findings".
|
||||
lands with its regression test.
|
||||
|
||||
1. **4.1 — restore `TestJobListViewIsCompact`, retire the other row.** Smallest,
|
||||
and it restores an enforcement STANDARDS calls mandatory. Do it first so the
|
||||
|
||||
@@ -1,86 +0,0 @@
|
||||
# GoSentry — Review Agenda
|
||||
|
||||
What to look at when reviewing the project as a whole, as opposed to a single
|
||||
diff. This is the agenda; the rules a review checks against live in
|
||||
[STANDARDS.md](STANDARDS.md) and [ARCHITECTURE.md](ARCHITECTURE.md).
|
||||
|
||||
Scope note: a normal pull-request review checks the change. This agenda is for
|
||||
a periodic sweep of the whole codebase, so a pass may legitimately end with
|
||||
"nothing to report" on most items.
|
||||
|
||||
## 1. Architecture and project structure
|
||||
|
||||
Does the code still match the package map and the event flow in
|
||||
[ARCHITECTURE.md](ARCHITECTURE.md)? Watch for the boundaries that matter here:
|
||||
`app.Service` as the sole owner of job and runtime state, the UI reading it
|
||||
through typed events, `domain` staying free of I/O, and platform-specific code
|
||||
staying behind the `platform/*` interfaces.
|
||||
|
||||
## 2. Complexity against the size of the project
|
||||
|
||||
GoSentry is a single-process desktop app with two direct dependencies. Flag
|
||||
abstraction that is not paying for itself: interfaces with one implementation
|
||||
and no test seam, indirection added for a use case nobody has asked for, a new
|
||||
dependency where thirty lines of standard library would do. Also check the
|
||||
opposite direction — files that have grown past the size guideline in
|
||||
ARCHITECTURE and should be split the way `jobs_view.go` was.
|
||||
|
||||
## 3. Code quality
|
||||
|
||||
The checkable rules are in [STANDARDS.md](STANDARDS.md) — error handling, unit
|
||||
tests for pure helpers, regression tests for fixes, `fyne.Do` for updates off
|
||||
the main thread. Beyond them: concurrency around `Service.mu`, goroutines whose
|
||||
lifetime is not obvious, and error paths that report something less useful than
|
||||
what they caught.
|
||||
|
||||
## 4. Documentation and comments
|
||||
|
||||
Does every documented behavior still exist, and does every non-obvious behavior
|
||||
get documented? Check the doc set against the code: README (user-facing
|
||||
behavior and config keys), ARCHITECTURE (packages and flows), STANDARDS
|
||||
(rules and intentional behavior), DEVELOPMENT (build), TESTS, PERFORMANCE,
|
||||
CHANGELOG (an entry per notable change). For comments, the bar is *why*, not
|
||||
*what* — a comment restating the line below it is noise; an unexplained
|
||||
workaround is a finding.
|
||||
|
||||
## 5. Readability and maintainability
|
||||
|
||||
Read a package as someone who has not seen it before. Can the next change be
|
||||
made without reverse-engineering? Naming that matches the domain vocabulary,
|
||||
functions that do one thing, and control flow that does not need a diagram.
|
||||
|
||||
## 6. Logical errors
|
||||
|
||||
Correctness independent of style: scheduling and timing edge cases (overlap
|
||||
policy, sequential mode, pause interactions), off-by-one and boundary handling,
|
||||
zero values that mean something (see the timeout rules in STANDARDS), state
|
||||
that can be observed mid-update, and error paths that leave state inconsistent.
|
||||
|
||||
## 7. Legacy code and migrations
|
||||
|
||||
The app has no database, so migration means file compatibility: `gosentry.json`
|
||||
and `jobs.json` written by an older version must keep working. Check that new
|
||||
`Config` fields are backward compatible, that normalization happens in one
|
||||
place, and that values which are meaningful zeros are not normalized away. Also
|
||||
look for code kept alive only for a case that no longer exists.
|
||||
|
||||
## 8. Undocumented or under-documented contentious decisions
|
||||
|
||||
Any decision a future reader would question needs its reasoning recorded where
|
||||
it lives: a comment at the code, an entry in the "Intentional behavior" section
|
||||
of [STANDARDS.md](STANDARDS.md), or — when the work is deferred rather than
|
||||
decided — a note in [ROADMAP.md](ROADMAP.md), which is where the frozen
|
||||
window-size work keeps its rationale.
|
||||
|
||||
## 9. Other improvement proposals
|
||||
|
||||
Anything that does not fit above: build and release ergonomics, test coverage
|
||||
gaps, dependency health, UX rough edges.
|
||||
|
||||
## What happens to the findings
|
||||
|
||||
- A defect → fix it, with a regression test when severity is medium or higher.
|
||||
- Behavior that turns out to be deliberate → record it under "Intentional
|
||||
behavior" in [STANDARDS.md](STANDARDS.md) so it is not re-reported.
|
||||
- Work larger than a single fix → [ROADMAP.md](ROADMAP.md), with the reasoning.
|
||||
- A new rule the review establishes → [STANDARDS.md](STANDARDS.md).
|
||||
+1
-1
@@ -136,7 +136,7 @@ were already split once:
|
||||
| `src/storage/store.go` | 265 |
|
||||
|
||||
This is deliberately deferred to the next whole-project review rather than done
|
||||
piecemeal: [REVIEW.md](REVIEW.md) already asks item 2 to look for exactly this,
|
||||
piecemeal: a future review already asks item 2 to look for exactly this,
|
||||
a split touches every reader of the file, and doing all six in one pass keeps
|
||||
the seams consistent instead of settling them six different ways. Splitting is
|
||||
also the kind of change that reads as pure movement while quietly dropping a
|
||||
|
||||
+1
-2
@@ -1,8 +1,7 @@
|
||||
# GoSentry — Standards
|
||||
|
||||
Quality rules and intentional behavior for contributors. Package contracts live
|
||||
in [ARCHITECTURE.md](ARCHITECTURE.md); test conventions in [TESTS.md](TESTS.md);
|
||||
what a whole-project review looks at, in [REVIEW.md](REVIEW.md).
|
||||
in [ARCHITECTURE.md](ARCHITECTURE.md); test conventions in [TESTS.md](TESTS.md).
|
||||
|
||||
## Code quality
|
||||
|
||||
|
||||
Reference in New Issue
Block a user