From: Jonathan Nieder Date: Mon, 05 Jan 2015 20:22:44 GMT Subject: Re: [PATCHv10 01/10] receive-pack.c: shorten the execute_commands loop over all commands Message-ID: <20150105202244.GL29365@google.com> In-Reply-To: <1420482355-24995-1-git-send-email-sbeller@google.com> Stefan Beller wrote: > --- a/builtin/receive-pack.c > +++ b/builtin/receive-pack.c [...] > @@ -1077,27 +1100,15 @@ static void execute_commands(struct command *commands, [...] > + if (shallow_update) > + assure_connectivity_checked(commands, si); Looking at this code alone, it seems like assure_connectivity_checked() is going to ensure that connectivity was checked, so that I can assume connectivity going forward. But the opposite is true --- it is a safety check that prints a warning and doesn't affect what I can assume. The factored-out function fails in what it is meant to do, which is to save the reader of execute_commands from having to look at the implementation of the parts they are not interested in. Would something like warn_if_skipped_connectivity_check() make sense? Jonathan