fix(args): handle --workdir include paths relative to repo root#1428
fix(args): handle --workdir include paths relative to repo root#1428SergioChan wants to merge 2 commits intoorhun:mainfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1428 +/- ##
==========================================
- Coverage 47.57% 47.37% -0.20%
==========================================
Files 24 24
Lines 2119 2128 +9
==========================================
Hits 1008 1008
- Misses 1111 1120 +9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Hey, thanks for the PR. This probably fixes the issue but I don't think we agreed to go with a fix like this, see #1369 (comment) It further complicates the logic and we probably want to attack this on a wider scale (by refactoring some parts and adding some test fixtures first.) cc @ognis1205 |
|
Thanks for the PR and for looking into this issue! Just to give some context. Another contributor is already working on the same problem in a different PR: For now I'm trying to contact the author of that PR since they opened it earlier, and I'd like to prioritize based on the timeline. Depending on how things evolve, we might still move forward with this PR or potentially collaborate on it. Also, sorry that the issue wasn't explicitly assigned to anyone. On my side, I'm planning to prepare some test fixtures soon to make it easier to work on this area: If you're interested in helping with that as well, feel free to take it. |
Thanks, that sounds great! I'll assign you to #1324 for the fixtures work 👍 |
|
Could you leave a quick comment on #1324 so I can assign you to it? |
|
Done — I just commented on #1324 and I’m ready to help with the fixtures/test coverage there. |
|
Pushed follow-up commit What changed
Validation
|
|
Hi @SergioChan cc: @o1x3 Sorry for the slow progress. The author of the earlier PR addressing the same issue hasn't responded yet. If you still intend to move forward with this PR, would you like to continue handling it? |
|
@ognis1205 @SergioChan sorry for the delay on my side. I had to take some time off for personal reasons, but I am back now. Thanks Sergio for carrying the issue while I was inactive. I reworked It also still makes sense to me for fixture work to stay on Happy to collaborate too if that would help. |
Description
Fixes
--workdirpath handling so repository-root workdirs no longer force an include filter that drops commits.Motivation and Context
Fixes #1369.
Previously,
--workdiralways injected an include glob from the raw workdir path. When workdir pointed to the repository root (or a parent directory in combinations like--repository ... --workdir ./), the generated include pattern did not match commit file paths, producing an empty changelog.This change:
args.include_pathearly from--workdir.--workdiragainst the discovered repo root later (inprocess_repository).That preserves subdirectory scoping while avoiding false-empty output for root workdirs.
How Has This Been Tested?
Local Rust toolchain is unavailable in this environment (
cargonot installed), so I could not runcargo test/clippyhere.Validation performed:
run_with_changelog_modifier+process_repository.git-cliff --workdir .from repo root: no include filter forced.git-cliff --workdir <repo-dir>from parent: no include filter forced.git-cliff --repository <repo-dir> --workdir ./from parent: no include filter forced.git-cliff --workdir <subdir>: include filtering still applied for subdirectory scope.Screenshots / Logs (if applicable)
N/A
Types of Changes
Checklist:
cargo +nightly fmt --allcargo clippy --tests --verbose -- -D warningscargo test