On December 17, 2018 5:57:50 PM EST, "Ævar Arnfjörð Bjarmason" <avarab@xxxxxxxxx> wrote: > >On Mon, Dec 17 2018, David Turner wrote: > >> Overall, I like this. One nit: > >Thanks for the review! > >> On December 17, 2018 5:16:25 PM EST, "Ævar Arnfjörð Bjarmason" ><avarab@xxxxxxxxx> wrote: >>>--- a/upload-pack.c >>>+++ b/upload-pack.c >>>@@ -54,7 +54,8 @@ static int no_progress, daemon_mode; >>> #define ALLOW_REACHABLE_SHA1 02 >>>/* Allow request of any sha1. Implies ALLOW_TIP_SHA1 and >>>ALLOW_REACHABLE_SHA1. */ >>> #define ALLOW_ANY_SHA1 07 >>>-static unsigned int allow_unadvertised_object_request; >>>+static unsigned int allow_unadvertised_object_request = ( >>>+ ALLOW_TIP_SHA1 | ALLOW_REACHABLE_SHA1 | ALLOW_ANY_SHA1); >> >> ALLOW_ANY_SHA1 already includes the other two. > >FWIW (and maybe I made the wrong call, and have no preference realy) I >opted for this as part of "this change doesn't do any of the hard work >of extracting the now-dead ALLOW_[...]*. > >I.e. the diff context I had doesn't show all the ALLOW_* values, and >with the context given it might be easier for reviewers to look no >further than "this is what you'd get before if all >uploadpack.allow.*SHA1InWant was true". The context I quoted said /* Allow request of any sha1. Implies ALLOW_TIP_SHA1 and ALLOW_REACHABLE_SHA1. */ But up to you (or Junio). -- Sent from my Android device with K-9 Mail. Please excuse my brevity.