feat: Submodule commit processing #1082
Conversation
|
Thanks for opening this pull request! Please check out our contributing guidelines! ⛰️ |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1082 +/- ##
==========================================
- Coverage 41.58% 40.70% -0.87%
==========================================
Files 21 21
Lines 1816 1865 +49
==========================================
+ Hits 755 759 +4
- Misses 1061 1106 +45
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:
|
|
Nice! So for a given commit range of the containing repo, this returns all submodules that changed within those commits, and their deltas between them? |
|
This is actually an improvement and "replaces" some of what I was trying to do, and I'm super happy to see it |
|
Well, this iteration sort of actually works! However, the submodule path is not correct ... I used openstack/openstack for testing btw, that repository does seem like the ultimate stresstest. |
|
So that is fixed as well, only config option, documentation and code cleanup/review are missing. |
|
This is some good stuff. Let me know when this is ready for review :) |
lehmanju
changed the title
There was a problem hiding this comment.
Looks nice!
Some things to do:
- Add fixture tests to ensure the functionality
- Update documentation about the new configuration option
|
Thanks for the review! I'll have time to address your comments in about a week. |
Co-authored-by: Orhun Parmaksız <orhunparmaksiz@gmail.com>
|
There does seem to be something wrong with the fixture tests in general: https://github.com/orhun/git-cliff/actions/runs/13907737767/job/38914649566#step:3:913 |
|
The more I test this in a real scenario the more corner cases I find: |
| @@ -64,6 +64,15 @@ following context is generated to use for templating: | |||
| "commit_id": "a440c6eb26404be4877b7e3ad592bfaa5d4eb210 (release commit)", | |||
| "timestamp": 1625169301, | |||
| "repository": "/path/to/repository", | |||
| "submodule_commits": { | |||
| "<relative/submodule_path/in/repository>": [ | |||
There was a problem hiding this comment.
Have you checked how this path is affected e.g. when running inside another directory in a monorepo?
There was a problem hiding this comment.
Do you mean, if git-cliff is invoked from a repository subdirectory that is not a submodule or from the submodule itself?
There was a problem hiding this comment.
added another fixture test for include-path. if git-cliff is invoked inside a submodule, it will only be able to process the submodule itself. if git-cliff is invoked inside a normal monorepo directory, it should work as expected. the path that is used to find the submodule is gathered from the commit itself, so it doesn't depend on where git-cliff is invoked.
Co-authored-by: Orhun Parmaksız <orhunparmaksiz@gmail.com>
|
@orhun If CI turns out good (which it should), this is ready for a (hopefully) final review.
|
So #1058 inspired me to try submodule support myself. It is not intended to be "better" or "replace" @yudjinn 's efforts. Since this feature is a blocker for me anyway I might as well put some time into it.