ci: add GitHub Pages coverage report and restructure workflows - #28
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds a GitHub Actions workflow that builds with MinGW, runs tests with coverage, generates an HTML coverage report, and publishes it to GitHub Pages; also renames a PR workflow display name, adjusts .gitignore coverage rules, updates README badges/links, modifies a coverage parsing script, and rewrites the test coverage guide. Changes
Sequence Diagram(s)mermaid Note over Actions,Checkout: Trigger (push to main / manual) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
.gitignore (1)
8-9: Prefer explicit allow-list for the nested ignore file.
docs/tests/coverage/works, but it’s safer to explicitly keepdocs/tests/coverage/.gitignoreunignored so the directory policy remains maintainable.Suggested `.gitignore` tweak
-# Coverage reports (generated into docs/tests/coverage/, has its own .gitignore) -docs/tests/coverage/ +# Coverage reports (generated into docs/tests/coverage/) +docs/tests/coverage/* +!docs/tests/coverage/.gitignore🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gitignore around lines 8 - 9, The ignore rule "docs/tests/coverage/" currently ignores the entire directory including its .gitignore, which makes managing per-directory ignores brittle; update .gitignore to explicitly allow the nested keep-file by adding an explicit negation for that file (e.g., add a line to unignore docs/tests/coverage/.gitignore) so the directory-level policy remains maintainable while keeping the coverage artefacts ignored..github/workflows/coverage-pages.yml (1)
47-49: Pingcovrto a tested version for CI reproducibility.The workflow uses
--gcov-ignore-parse-errors=negative_hits.warn_once_per_file(introduced in gcovr 6.0), so pinning to at leastpip install "gcovr>=6.0"ensures both consistent behavior and feature support. Unpinned installs risk breakage when new releases change CLI behavior—latest version 8.6 was released in January 2026.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/coverage-pages.yml around lines 47 - 49, Update the CI step that currently runs the command `pip install gcovr` (the step named "Install gcovr") to pin a tested version that supports the `--gcov-ignore-parse-errors=negative_hits.warn_once_per_file` flag; change the command to install a minimum version, e.g. `pip install "gcovr>=6.0"`, so CI behavior is reproducible and robust against upstream CLI changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/coverage-pages.yml:
- Around line 51-57: The workflow currently caches the CMake build directory
("Cache CMake build" step) at path build/mingw-debug using the key
cmake-build-${{ runner.os }}-... which can reuse stale coverage artifacts;
remove or disable this cache step (or change it to exclude any coverage output)
from the coverage/publish job so published coverage is always generated from a
fresh build, ensuring accuracy when running the coverage/publish workflow.
In `@docs/tests/parse_coverage.py`:
- Around line 4-7: The fallback coverage_path uses a CWD-relative literal which
breaks when the script is run from other directories; change the coverage_path
assignment to compute a path relative to the script file (use __file__ and
pathlib.Path.resolve().parent) and join it with 'coverage/coverage.json' so the
default points to the repo docs/tests/coverage file regardless of CWD, and
ensure pathlib.Path is imported if not already; keep handling of sys.argv[1]
unchanged and then open that resolved path before calling json.load.
In `@docs/tests/test_coverage_guide.md`:
- Around line 35-37: The gcovr invocation uses --filter "include/DetourModKit/"
which differs from CI's --filter "include/" and can cause inconsistent coverage;
update the gcovr commands (the lines containing python -m gcovr --root .
--filter "src/" --filter "include/DetourModKit/" --exclude ...) to use the same
--filter "include/" as CI (and make the same replacement in the other
occurrences noted near the same sections), ensuring local and CI coverage
filters match exactly.
In `@README.md`:
- Line 198: Update the sentence that currently reads "a 80% minimum line
coverage gate" to use the correct article: change "a 80% minimum line coverage
gate" to "an 80% minimum line coverage gate" in the README text (the line
mentioning CI PR Check workflow and coverage gate).
---
Nitpick comments:
In @.github/workflows/coverage-pages.yml:
- Around line 47-49: Update the CI step that currently runs the command `pip
install gcovr` (the step named "Install gcovr") to pin a tested version that
supports the `--gcov-ignore-parse-errors=negative_hits.warn_once_per_file` flag;
change the command to install a minimum version, e.g. `pip install
"gcovr>=6.0"`, so CI behavior is reproducible and robust against upstream CLI
changes.
In @.gitignore:
- Around line 8-9: The ignore rule "docs/tests/coverage/" currently ignores the
entire directory including its .gitignore, which makes managing per-directory
ignores brittle; update .gitignore to explicitly allow the nested keep-file by
adding an explicit negation for that file (e.g., add a line to unignore
docs/tests/coverage/.gitignore) so the directory-level policy remains
maintainable while keeping the coverage artefacts ignored.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 084106ae-cd15-452f-be1e-971dcfc30298
📒 Files selected for processing (6)
.github/workflows/coverage-pages.yml.github/workflows/pr-check.yml.gitignoreREADME.mddocs/tests/parse_coverage.pydocs/tests/test_coverage_guide.md
Summary
coverage-pages.ymlworkflow: builds, tests, generates HTML coverage via gcovr, and deploys to GitHub Pages on every push tomain.ci.yml→pr-check.ymlto clarify its role as the PR gate (no functional changes).docs/tests/coverage/with its own.gitignoreto prevent stale file accumulation.parse_coverage.pyto accept an optional path argument (defaults to new location).docs/tests/test_coverage_guide.mdto match current codebase (fix broken fences, correct file names/line counts, add integration test docs, update all commands).Live coverage
https://tkhquang.github.io/DetourModKit/
Test plan
parse_coverage.pyworks with new default pathtest_coverage_guide.mdverifiedSummary by CodeRabbit
New Features
Documentation
Tests
Chores