Formal review of all 13 review-sets; apply findings#38
Conversation
Agent-Logs-Url: https://github.com/demaconsulting/ReviewMark/sessions/78f46014-06de-48bf-85c7-0edbb7983736 Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
|
@copilot Please review the README.md, docs/user_guide/introduction.md, docs/reqstream/reviewmark-system.yaml, docs/design/introduction.md and docs/design/system.md files and determine whether the requirements and design of reviewmark accurately reflect what's being advertised to the user. |
Agent-Logs-Url: https://github.com/demaconsulting/ReviewMark/sessions/3a13bcb6-e2e3-4c73-9f48-566e4c5c8f96 Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
Cross-checked the five files. Two gaps found and fixed (commit dba0ff5):
Everything else in the five files accurately reflects the implementation:
|
There was a problem hiding this comment.
Pull request overview
Updates project documentation and test assertions based on a formal pass over all configured review-sets, aligning docs/requirements with actual CLI/config behavior and tightening MSTest exception assertions.
Changes:
- Updated CLI/design docs to reflect current
Program.Run()dispatch behavior andContextproperty semantics. - Corrected user guide lint documentation for
evidence-sourcesupported types andlocationrequirements. - Tightened CLI argument validation tests by switching to
Assert.ThrowsExactly<ArgumentException>().
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/DemaConsulting.ReviewMark.Tests/Cli/ContextTests.cs | Uses ThrowsExactly for stricter exception-type assertions in CLI parsing tests. |
| docs/user_guide/introduction.md | Updates lint checks description for evidence-source values and location requirement. |
| docs/reqstream/reviewmark-system.yaml | Updates system requirement justifications to include Failed status and enforcement behavior. |
| docs/design/program.md | Corrects Run() dispatch narrative; separates PrintBanner() vs PrintHelp() behavior. |
| docs/design/cli/context.md | Updates Context property table (ElaborateId) to match implementation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@copilot apply changes based on the comments in this thread |
…nforce justification Agent-Logs-Url: https://github.com/demaconsulting/ReviewMark/sessions/5adf8c75-f585-4404-b78d-e5314af3906c Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
Applied in commit b84f71e:
|
Ran the
@code-reviewagent against each of the 13 review-sets defined in.reviewmark.yaml, processing each separately. Seven actionable findings were identified and fixed.Description
docs/design/cli/context.mdElaborate | bool→ElaborateId | string?to match actual implementationWorkingDirectory | string→WorkingDirectory | string?to match actual implementation (null when--diris not provided)docs/design/program.md--version);--indexis not a standalone priority but part ofRunToolLogicPrintBanner()andPrintHelp()descriptions accurately; prior doc conflated the two methodstest/DemaConsulting.ReviewMark.Tests/Cli/ContextTests.csAssert.Throws<ArgumentException>()withAssert.ThrowsExactly<ArgumentException>()per MSTest V4 standards — the looser assertion would silently pass if a derived exception type (e.g.ArgumentNullException) were thrown insteaddocs/user_guide/introduction.mdnonewas omitted from the list of validevidence-sourcetype values (none,url,fileshare)evidence-sourceblock description to makelocationconditional: required only forurl/fileshare, not used withnonedocs/reqstream/reviewmark-system.yamlFailedto theReviewMark-System-ReviewReportjustification — the tool emits four statuses (Current, Stale, Missing, and Failed), not threeReviewMark-System-Enforcejustification to cover both enforcement triggers: uncovered files in the Review Plan (files matchingneeds-reviewnot covered by any review-set) and non-Current statuses in the Review Report (Stale, Missing, or Failed)Type of Change
Related Issues
Pre-Submission Checklist
Before submitting this pull request, ensure you have completed the following:
Build and Test
dotnet build --configuration Releasedotnet test --configuration Releasedotnet run --project src/DemaConsulting.ReviewMark --configuration Release --framework net10.0--no-build -- --validateCode Quality
dotnet format --verify-no-changesQuality Checks
Please run the following checks before submitting:
cspell "**/*.{md,cs}"markdownlint "**/*.md"yamllint .Testing
Documentation
Additional Notes
All 13 review-sets passed. The remaining reviews (ReviewMark-ReviewMarkConfiguration, ReviewMark-GlobMatcher, ReviewMark-ReviewIndex, ReviewMark-PathHelpers, ReviewMark-Validation, ReviewMark-Cli, ReviewMark-Indexing, ReviewMark-SelfTest, ReviewMark-System, ReviewMark-Design, ReviewMark-AllRequirements) had no blocking issues.