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".
