feat: Make cz bump work on shallow clones
#649
Conversation
This allows reliably setting up your CI to only fetch the latest changes. E.g. Gitlab CI uses shallow clones with a depth of 20 by default: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/77576 Only `tag_exists()` had to be modified for this. `cz changelog` is not ready yet, more functions would have to include a `git fetch` for that. I was worried about slowing down error reporting in non-CI workflows ("git fetch" calls can sometimes take a while if your connection is bad). To minimize the risk for this, now only fetching the tag for shallow clones and always printing a warning when this happens. I was hit by this issue while trying out Commitizen in Gitlab CI on a monorepo, where some packages haven't seen a release for >50 commits.
Codecov ReportBase: 97.92% // Head: 98.10% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #649 +/- ##
==========================================
+ Coverage 97.92% 98.10% +0.18%
==========================================
Files 35 39 +4
Lines 1252 1690 +438
==========================================
+ Hits 1226 1658 +432
- Misses 26 32 +6
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
It doesn't look like the CI tests failed because of my change. Please let me know if that's the case in your opinion. |
|
@robertschweizer I just reran the CI and it passed. Will take a deeper look. Thanks! |
| capsys.readouterr() # Reset capsys fixture | ||
| cli.main() | ||
| stdout, _stderr = capsys.readouterr() | ||
| assert re.search("Could not find tag.*trying to fetch", stdout) |
There was a problem hiding this comment.
Should we also check whether the function returns correctly after fetching?
| if tag in c.out: | ||
| return True | ||
|
|
||
| # In shallow clones (e.g. set up by Gitlab CI), the previous release tag might not |
There was a problem hiding this comment.
I'm good with this functionality. But one of the concerns I have is whether this is an unexpected action from the user point of view. I'm thinking of using a flag.
There was a problem hiding this comment.
Agreed, this is more CI configuration, is up to the user, I'm fine with a warning and an explanation with the next steps to the user. But the user should ensure the environment (git) is providing commitizen what's needed, not the other way around.
There was a problem hiding this comment.
Thanks for the reviews!
I agree with your concerns that auto-fetching missing tags is unexpected from the user point of view. Actually it would be cleanest to me if git tag --list {tag} had a flag to do this fetching for shallow clones automatically. But without auto-fetching missing tags in shallow clones, the only option would be to do a full clone, which can be quite slow for large/old repositories.
I believe both Gitlab CI and Github Actions use shallow clones by default. Because of that, Commitizen should ideally handle shallow clones out-of-the-box without setting an extra flag. Do you have examples which workflows could break if we fetch in shallow clones by default? I'm mostly worried about connection issues/the CI not setting up the Git remote for later fetching.
If you keep not being convinced, should we call the flag --auto-fetch and only add it for the cz bump command for now?
There was a problem hiding this comment.
Hi @robertschweizer Sorry for late reply. I like the idea of the flag --auto-fetch. @woile What do you think?
There was a problem hiding this comment.
Yes, a flag and a setting. We can prompt the user in the init command
Lee-W
added
pr-status: wait-for-review
discussion
issue-status: wait-for-response
issue-status: wait-for-implementation
|
Thanks a lot for your review and feedback! I'm sorry about this, but I actually don't need this change anymore. In our fork we actually restrict Feel free to close this MR unless someone else finds it useful. |
Description
Try to
git fetchmissing tags if working on a shallow clone.This allows reliably setting up your CI to only fetch the latest changes. E.g. Gitlab CI uses shallow clones with a depth of 20 by default: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/77576
Only
tag_exists()had to be modified to make this work forcz bump. Other functions are not ready yet, more functions would have to include agit fetchfor that. There are also other problems still with Gitlab CI's shallow clones, e.g. #593. But for my use-case of just runningcz bumpandcz checkin Gitlab CI this works nicely.Checklist
./scripts/formatand./scripts/testlocally to ensure this change passes linter check and testAdditional context
I was worried about slowing down error reporting in non-CI workflows ("git fetch" calls can sometimes take a while if your connection is bad). To minimize the risk for this, I'm now only fetching the tag for shallow clones and always printing a warning when this happens.
I was hit by this issue while trying out Commitizen in Gitlab CI on a monorepo, where some packages haven't seen a release for >50 commits. It's also large enough for the shallow clone to really make sense in the CI.