On Sun, Jun 19, 2016 at 06:09:28PM -0700, Stefan Beller wrote:

> > I hadn't paid much attention to this topic originally, but was surprised
> > that "--depth 10" in the clone implies "--depth 1" in the submodule.
> > This is not really related to your patch (in fact, your patch makes the
> > logic go away). But maybe something to consider if it's ever resurrected
> > (or possibly if somebody runs "--shallow-submodules --depth 5" we should
> > pass --depth=1; I dunno).
> 
> How often do we see a depth != 1 in practice?
> I have the impression (and no data to back up my claim) that a binary
> switch for nonshallow or depth 1 would serve us just as good, which is why
> I did not want to ad complexity to the submodule depth.
> (What if you want submodule A with depth 2 and B with 5? In that
> case get them all shallow and deepen as appropriate, would be my answer)

To be honest, I don't know why people use anything except --depth=1, but
it's clear from my experience that they do. This example has --depth=10,
and on the server side at GitHub I have seen similar numbers from clients,
especially CI services.

(I take special note of such cases because --shallow quite often causes
performance problems on the server side, though generally --depth=10 is
not any worse than --depth=1. The worst case is really
"--no-single-branch --depth=1", which wants a ton of objects but has to
throw away all of the on-disk deltas).

> > We are not really testing "does not imply" here, but "passing
> > --shallow-submodules works". The "does not imply" test would be cloning
> > without the option and checking that the resulting submodules are not
> > shallow.
> 
> In case we want to be sure that it works for 2.9.1, i.e. we treat it
> as a regression,
> we need to test the "does not imply" a bit more I would think. I can send that
> test on top tomorrow if you'd like to.

I think it's worth doing (and testing both: the default behavior, and
that the --shallow-submodules feature works). Thanks.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to