feat(util/git): support checkout depth in submodules#26777
feat(util/git): support checkout depth in submodules#26777squat wants to merge 1 commit intoargoproj:masterfrom
Conversation
❗ Preview Environment deployment failed on BunnyshellSee: Environment Details | Pipeline Logs Available commands (reply to this comment):
|
ea2b708 to
2ec4076
Compare
|
I'm pretty sure the failure in the unit test is a flake because the test is timing out while trying to clone from github; this is likely hitting network issues or rate limits. |
8759ec2 to
fa5cc24
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #26777 +/- ##
==========================================
+ Coverage 63.04% 63.05% +0.01%
==========================================
Files 414 414
Lines 56286 56294 +8
==========================================
+ Hits 35484 35496 +12
Misses 17427 17427
+ Partials 3375 3371 -4 ☔ View full report in Codecov by Sentry. |
|
@squat, I like the idea of propagating the Have you considered refactoring the depth into one of the client options (would require changing |
choejwoo
left a comment
There was a problem hiding this comment.
+1 to @olivergondza suggestion.
Would it be cleaner to make depth a client option, since it is repo-level config, instead of threading it through multiple methods?
be38c86 to
e990b47
Compare
|
@olivergondza @choejwoo thanks for taking a look at this changeset! On the subject of the specifying the clone depth for the client, agreed, I think that setting the depth once as an option on the |
This commit adds support for configuring the checkout depth of submodules. The simple approach here in this PR is to use the global depth configured for the repository for the submodule. In my opinion, this is the most logical way to treat the current configuration surface, which only exposes a single field to configure the checkout depth of a monorepo. Rather than add more knobs to tune the checkout depth of every single different submodule in a repo, this commit takes the simple approach of applying the depth globally to all submodules. The commit introduces new unit tests to validate that shallow checkouts work for submodules. Notably, this commit changes the signature of the `Fetch` method on the Git client interface to no longer accept an argument for the checkout depth. Instead, the depth is now configured as an option massed to the Git client initializer: `WithDepth(<depth>)`. Fixes: argoproj#26774 Signed-off-by: squat <lserven@gmail.com>
e990b47 to
40cf85e
Compare
olivergondza
left a comment
There was a problem hiding this comment.
Perfect, looks nicer!
This commit adds support for configuring the checkout depth of
submodules. The simple approach here in this PR is to use the global
depth configured for the repository for the submodule.
In my opinion, this is the most logical way to treat the current
configuration surface, which only exposes a single field to configure
the checkout depth of a monorepo. Rather than add more knobs to tune
the checkout depth of every single different submodule in a repo, this
commit takes the simple approach of applying the depth globally to all
submodules.
The commit introduces new unit tests to validate that shallow checkouts
work for submodules.
Fixes: #26774
Signed-off-by: squat lserven@gmail.com