Fix checklist format, active voice, and skill routing in markdown files#2979
Fix checklist format, active voice, and skill routing in markdown files#2979
Conversation
Co-authored-by: afranken <763000+afranken@users.noreply.github.com>
Co-authored-by: afranken <763000+afranken@users.noreply.github.com>
…correctly Co-authored-by: afranken <763000+afranken@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR standardizes contributor/agent documentation by converting checklists to checkbox format and active-voice imperatives, refines .claude skill descriptions to reduce routing ambiguity between implement and document, and updates build/test docs to use new make targets instead of raw ./mvnw commands.
Changes:
- Convert multiple checklist sections to
- [ ]format and rewrite items in active voice (imperative verbs). - Adjust
.claudeskill descriptions so documentation-only tasks route todocument, notimplement. - Add Makefile convenience targets (
run,test,format,integration-tests,skip-docker) and update docs to reference them.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
server/AGENTS.md |
Updates run instructions to use make run. |
integration-tests/AGENTS.md |
Updates test instructions to use make integration-tests. |
docs/TESTING.md |
Switches common commands to make targets; converts checklist phrasing to imperative voice. |
README.md |
Updates build/run snippets to make targets. |
Makefile |
Adds skip-docker, format, integration-tests, run, test targets and updates .PHONY. |
AGENTS.md |
Updates build section to use make targets. |
.github/PULL_REQUEST_TEMPLATE.md |
Updates formatting command to make format. |
.github/CONTRIBUTING.md |
Updates build/format instructions to make targets and refreshes wording/formatting. |
.claude/skills/test/SKILL.md |
Converts checklist items to imperative voice and updates formatting command to make format. |
.claude/skills/refactor/SKILL.md |
Converts checklist items to imperative voice and updates formatting command to make format. |
.claude/skills/implement/SKILL.md |
Narrows scope to source-code changes and updates checklist/troubleshooting commands to make targets. |
.claude/skills/document/SKILL.md |
Broadens doc-scope description and converts checklist bullets to checkboxes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| **Requirements**: Java 17+, Maven 3.9+, Docker (for Docker build and integration tests) | ||
|
|
||
| **Build:** | ||
| ```shell | ||
| # Full build with Docker | ||
| ./mvnw clean install | ||
|
|
||
| # Skip Docker build | ||
| ./mvnw clean install -DskipDocker | ||
| make install # Full build with Docker | ||
| make skip-docker # Skip Docker build |
There was a problem hiding this comment.
The README lists build requirements as “Java 17+”, but the repo is configured to compile with Java 25 (see pom.xml <java.version>25</java.version>) and CONTRIBUTING.md also calls out Java 25. Please align README’s requirements with the actual build toolchain (e.g., Java 25 for compiling, targeting JVM 17 if that’s the intent) to avoid misleading contributors/users.
| @@ -25,5 +25,20 @@ verify: | |||
| install: | |||
| ./mvnw -B -V -Dstyle.color=always clean install | |||
|
|
|||
| skip-docker: | |||
| ./mvnw -B -V -Dstyle.color=always clean install -DskipDocker | |||
|
|
|||
| format: | |||
| ./mvnw -B -V -Dstyle.color=always ktlint:format | |||
|
|
|||
| integration-tests: | |||
| ./mvnw -B -V -Dstyle.color=always verify -pl integration-tests | |||
|
|
|||
| run: | |||
| ./mvnw spring-boot:run -pl server | |||
|
|
|||
| test: | |||
| ./mvnw -B -V -Dstyle.color=always test -pl server | |||
There was a problem hiding this comment.
The PR title/description focuses on markdown checklist/voice and skill routing, but this change also introduces new Makefile targets (and other docs now rely on them). Please update the PR description/title to reflect the new build-command abstraction, or consider splitting the Makefile + command migration into a separate PR so the scope is clear for reviewers.
Checklist items in several skill/docs files used plain bullets instead of
- [ ]checkboxes, and were written as passive statements rather than active-voice imperatives. Additionally, theimplementskill was being invoked for documentation tasks due to overlapping/ambiguous skill descriptions.Checklist format & active voice (
docs/,.claude/skills/)document/SKILL.md: plain-bullets →- [ ]checkboxesrefactor/SKILL.md,test/SKILL.md,docs/TESTING.md: passive statements converted to imperative verbsTests pass locally→Verify tests pass locally,Assertions are specific→Use specific assertionsSkill routing (
document/SKILL.md,implement/SKILL.md)implementmatched documentation tasks because its description included "fix bugs / modify code" (too broad), whiledocumentonly listed "document code, create docs, explain features" (too narrow to match fix/edit requests).implement: scoped to source code only; adds explicit exclusion:Not for documentation-only changesdocument: extended to coverfix formatting, wording, or style in existing documentation files💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.