Repository navigation
fix: resolve a relative --database against the working directory - #537
Conversation
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Resolve relative database paths cpp-linter/src/cli/structs.rs, cpp-linter/tests/comments.rs, cpp-linter/tests/reviews.rs |
ClangParams::from resolves relative database paths against the current working directory and retains the supplied path if resolution fails. The repository root is cloned from CLI options. Tests check the resolution behavior and pass absolute database paths. |
Estimated code review effort: 2 (Simple) | ~10 minutes
Possibly related issues
- Issue 514 tracks differences from cpp-linter v1 before GA. This change addresses the relative
--databaseresolution difference.
Merge Risk: 
37b34
-
The database path behavior and updated tests are consistent with the intended change.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title states the main change: a relative --databasepath is now resolved against the working directory. The raw summary confirms thatClangParams::fromresolves a relative compilation-database…Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Title check
-
Explanation
The title states the main change: a relative
--databasepath is now resolved against the working directory. The raw summary confirms thatClangParams::fromresolves a relative compilation-database path against the current working directory instead of the repository root. The title uses the "fix:" prefix, which fits the behavior change. The title is specific, short, and matches the changeset and the PR objectives.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 3 functions across 3 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #537 +/- ##
=======================================
Coverage 99.82% 99.82%
=======================================
Files 29 29
Lines 7432 7460 +28
=======================================
+ Hits 7419 7447 +28
Misses 13 13 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
<!-- claude-projects-attribution: {"github_login":"shenxianpeng"} -->
As suggested in
#535 (comment):
a new `--gen-patch-path <PATH>` option (also read from the
`GEN_PATCH_PATH` env var) sets where the combined fix patch is written,
so users can run `git apply "$GEN_PATCH_PATH"` without copying the path
from the logs.
- Setting it implies `--gen-patch`.
- A relative path is relative to the current working directory (same as
`--database` in #537, and what `git apply` expects). Note that
`--summary-output-file` and `--sarif-file` still resolve against
`--repo-root`; happy to switch this one to match those if you prefer.
- Without it, the patch stays at
`<repo-root>/.cpp-linter-cache/auto-fix.patch`, and `fix-patch-path`
reports whichever path was used.
- A stale patch at the chosen path is removed at the start of a run,
like the default one.
Implementation:
- `FeedbackInput::patch_path` holds the resolved path;
`ClangParams::default_patch_path()` builds the default.
- `FileObj::maybe_append_patch()` and `make_suggestions_from_patch()`
take the patch path instead of building it from `repo_root` (both are
`pub`, so this changes their signatures).
- Workspace `clap` enables the `env` feature (no new crates in
`Cargo.lock`).
- Tests: `cli::structs::test::{default_patch_path,
gen_patch_path_implies_gen_patch, gen_patch_path_from_env}` and
`run::test::custom_patch_path` (stale file replaced, nothing written to
the default path).
Checked locally with clang 21: all `cpp-linter` lib tests (120), `--test
comments` (17) and `--test reviews` (16) pass single-threaded (plain
`cargo test` races on env vars, nextest doesn't); `cargo clippy
--all-features --all-targets` clean; the generated CLI doc renders the
option and its example block.
---------
Co-authored-by: Xianpeng Shen <3353385+shenxianpeng@users.noreply.github.com>
2bndy5
left a comment
2bndy5
left a comment
There was a problem hiding this comment.
Need to rebase branch to fix CI checks
As suggested in #535 (comment): a relative
--databasepath is now resolved against the current working directory, like cpp-linter v1 (1.15+), instead of against--repo-root. Results only differ when--repo-rootis not..ClangParams::from(&Cli)usesstd::path::absolute()(no filesystem access, so a missing path still reaches the existing "Failed to read compilation database" handling).cli::structs::test::relative_db_path(--repo-root sub --database buildgives<cwd>/build).tests/comments.rsandtests/reviews.rspassed-p=buildtogether with--repo-root=<tmp dir>. With this change that path pointed atcpp-linter/build, so the tests still passed but without the meson database (each run logged "Failed to read compilation database file"). They now pass the absolute<tmp dir>/build.Checked locally (clang 21):
cargo test --all-features --lib db_path,--test comments(17 passed, single-threaded) and--test reviews(16 passed), no "Failed to read compilation database" warning;cargo clippy --all-features --all-targetsclean,cargo fmt.Summary by CodeRabbit