update implementation plan based on recent work#709
Conversation
Generated-by: Claude (Opus 4.8)
choo121600
left a comment
There was a problem hiding this comment.
LGTM — clean planning-doc refresh: the plan is rewritten to track only open gaps, the In-flight table now carries verifiable Implemented by commits, the new maintainer-education.md spec is well-formed, every cross-reference resolves, and CI is green. Two nits below, neither gating.
I checked the things a docs/planning change can actually get wrong:
- No dangling links introduced — every path the new spec points at exists on
main:docs/index.md,docs/rfcs/RFC-AI-0004.md,tools/privacy-llm/pii.md,tools/skill-evals/, plus the MISSION/PRINCIPLES anchors. - Spec matches the sibling convention —
specs/maintainer-education.mdcarries the SPDX header and the same frontmatter shape asspecs/skill-reconciler.md/specs/reviewer-routing.md(title/status/kind/mode/source/acceptance), and the standard section set (What it does / Where it lives / Behaviour & contract / Out of scope / Acceptance criteria / Validation / Known gaps). - Internally consistent — work items 2–6 correctly realize "one landing page plus one per MISSION-named piece" (the four pieces: pattern catalogue, your-first-skill, eval-driven development, workshops); item 7 is properly flagged as an epic that must be decomposed before building; and the new
overview.mdrow (proposed — release-blocking per PRINCIPLE 18) is consistent with the spec'sstatus: proposed.
Smaller observations
- PR body
## Summaryis empty. For a change that deletes 384 lines and re-scopes the whole work queue, a one-line "why now" — the merge train shipped the old items 1–22, so the plan refocuses on mail-privacy README compliance and the release-blocking education stream — would help future readers of the plan's history. Nit, not blocking. .typos.toml—extend-ignore-re = ["[0-9a-f]{7,40}"]. Correctly scoped to backtick-wrapped hex and pairs well with the newImplemented bycommit column. Just noting it will also silence a genuine ≥7-char all-hex token that happens to be backtick-wrapped (e.g. a stray`deadbeef`-style word) — vanishingly rare in prose, so fine as-is.
This review was drafted by an AI-assisted tool and confirmed by an Apache Magpie maintainer. The maintainer approving this PR has read the findings and signed off. If something feels off, please reply on the PR and a maintainer will follow up.
More on how Apache Magpie handles contributions: CONTRIBUTING.md.
|
looks good :) |
potiuk
left a comment
There was a problem hiding this comment.
LGTM — docs-only plan refresh. The plan correctly stops mirroring shipped state (deferring that to specs/, code, and git history) and re-centres on the maintainer-education stream; the new .typos.toml ignore rule matches the in-flight table's backtick-hash column exactly, and the new specs/maintainer-education.md carries its SPDX header and stays placeholder-clean. CI green. (Second maintainer sign-off — already approved by @choo121600.)
Summary
Type of change
.claude/skills/<name>/) — eval fixtures updated belowtools/<system>/*.md)tools/*/withpyproject.toml)docs/,README.md,CONTRIBUTING.md)projects/_template/)prek, workflows, validators)Test plan
prek run --all-filespassesuv run pytest/ruff check/mypypasses(
PYTHONPATH=tools/skill-evals/src python3 -m skill_evals.runner tools/skill-evals/evals/<skill>/)(a regression test for the bug fixed / the behaviour added — see CONTRIBUTING.md)