Hi Junio, On Wed, 24 Oct 2018, Junio C Hamano wrote: > "Johannes Schindelin via GitGitGadget" <gitgitgadget@xxxxxxxxx> > writes: > > > diff --git a/builtin/repack.c b/builtin/repack.c > > index c6a7943d5..9217fc832 100644 > > --- a/builtin/repack.c > > +++ b/builtin/repack.c > > @@ -549,6 +549,12 @@ int cmd_repack(int argc, const char **argv, const char *prefix) > > if (!po_args.quiet && isatty(2)) > > opts |= PRUNE_PACKED_VERBOSE; > > prune_packed_objects(opts); > > + > > + if (!keep_unreachable && > > + (!(pack_everything & LOOSEN_UNREACHABLE) || > > + unpack_unreachable) && > > + is_repository_shallow(the_repository)) > > + prune_shallow(0, 1); > > The logic looks correct (and the reasoning behind it, which was > explained in the log message, was sound). prune_shallow(0, 1) > however is not easily understandable. > > There are only two callsites of this function after these three > patches, and the areas of the code that have these calls are in > relatively quiescent parts in the whole tree, so it shouldn't be too > distracting to do an update to make the function take a flag word, > so that we can make callsites more immediately obvious, i.e. > > prune_shallow(PRUNE_SHALLOW_QUICK) > > It of course can be left as a low-hanging fruit loose-end. I almost did that, but then decided not to clutter the previous patch with this change (and global constant). Having said that, since you also had this idea, I will make that change. It will read nicer, you are right. Ciao, Dscho > > > diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh > > index 2d0031703..777c9d1dc 100755 > > --- a/t/t5537-fetch-shallow.sh > > +++ b/t/t5537-fetch-shallow.sh > > @@ -186,7 +186,7 @@ EOF > > test_cmp expect actual > > ' > > > > -test_expect_failure '.git/shallow is edited by repack' ' > > +test_expect_success '.git/shallow is edited by repack' ' > > git init shallow-server && > > test_commit -C shallow-server A && > > test_commit -C shallow-server B && >
