Skip to content

Feat/36/review lane enforces conventions skill list - #50

Merged
Kripu77 merged 2 commits into
mainfrom
feat/36-review-conventions
Aug 27, 2026
Merged

Feat/36/review lane enforces conventions skill list#50
Kripu77 merged 2 commits into
mainfrom
feat/36-review-conventions

Conversation

@michaelnaughton-euc

Copy link
Copy Markdown
Collaborator

The review lane now gets the .factory/conventions skill list in its rules whenever the file exists in the target checkout, phrased for reviewing: invoke each listed skill and flag every violation as a review comment. lanes/review.md carries the matching instruction, and tests/conventions.sh covers both the injected rules and the missing-file case. Closes #36.

@michaelnaughton-euc michaelnaughton-euc left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thermo-nuclear code quality review of this PR only. Behavior looks correct and the tests cover both the injected-rules and missing-file paths well. No blockers, but there is a code-judo move here that would delete the new branching instead of adding it — details inline. Summary of findings:

  1. The review imperative now lives in two homes: lanes/review.md:6 and the header injected by conventions_rules. Implementing lanes keep that imperative in exactly one place (the helper). Pick one home.
  2. The new lane mode param on conventions_rules exists only to swap prose. If the imperative sentences move into the lane playbooks, the helper becomes lane-agnostic data emission and the param, the branch, and the duplicated "This repo enables these skills; invoke each one that applies" line all disappear.
  3. Minor: run_review in tests copies run_lane's env boilerplate; the RULES+CONVENTIONS assembly is now duplicated verbatim between the feature|bug|docs and review case arms.

Also noting for the record: factory.sh was already over 1000 lines before this PR (1025 → 1035), so this isn't a threshold crossing, but the duplicated rules-assembly in the case arms is the kind of growth that will keep pushing it up.

Comment thread factory.sh Outdated
Comment thread lanes/review.md
Comment thread tests/conventions.sh Outdated
@michaelnaughton-euc
michaelnaughton-euc marked this pull request as ready for review August 27, 2026 05:07
@Kripu77
Kripu77 merged commit dc3863a into main Aug 27, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Review lane enforces the per-repo conventions skill list

2 participants