All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Stefan Beller <sbeller@google.com>
Cc: pclouds@gmail.com, gitster@pobox.com, git@vger.kernel.org,
	sunshine@sunshineco.com, mhagger@alum.mit.edu,
	ronniesahlberg@gmail.com
Subject: Re: [PATCHv10 01/10] receive-pack.c: shorten the execute_commands loop over all commands
Date: Mon, 5 Jan 2015 12:22:44 -0800	[thread overview]
Message-ID: <20150105202244.GL29365@google.com> (raw)
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

  parent reply	other threads:[~2015-01-05 20:22 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-30 23:41 [PATCH 0/9] atomic pushes Stefan Beller
2014-12-30 23:41 ` [PATCHv9 1/9] receive-pack.c: shorten the execute_commands loop over all commands Stefan Beller
2015-01-03  2:20   ` Jonathan Nieder
2015-01-03  9:53     ` Duy Nguyen
2015-01-05 18:02       ` Stefan Beller
2015-01-05 18:25         ` [PATCHv10 01/10] " Stefan Beller
2015-01-05 18:25           ` [PATCHv10 02/10] receive-pack.c: die instead of error in assure_connectivity_checked Stefan Beller
2015-01-05 20:17             ` Jonathan Nieder
2015-01-05 21:15               ` Stefan Beller
2015-01-05 21:25                 ` Jonathan Nieder
2015-01-06 19:40                   ` [PATCHv11 02/11] receive-pack.c: die instead of error in case of possible future bug Stefan Beller
2015-01-06 19:46                     ` Jonathan Nieder
2015-01-05 20:22           ` Jonathan Nieder [this message]
2015-01-05 21:07             ` [PATCHv10 01/10] receive-pack.c: shorten the execute_commands loop over all commands Stefan Beller
2015-01-05 21:18               ` Jonathan Nieder
2015-01-06 19:34                 ` [PATCHv11 01/11] " Stefan Beller
2014-12-30 23:41 ` [PATCHv9 2/9] receive-pack.c: move iterating over all commands outside execute_commands Stefan Beller
2014-12-30 23:41 ` [PATCHv9 3/9] receive-pack.c: move transaction handling in a central place Stefan Beller
2014-12-30 23:41 ` [PATCHv9 4/9] receive-pack.c: add execute_commands_atomic function Stefan Beller
2014-12-30 23:41 ` [PATCHv9 5/9] receive-pack.c: negotiate atomic push support Stefan Beller
2014-12-30 23:41 ` [PATCHv9 6/9] send-pack: rename ref_update_to_be_sent to check_to_send_update Stefan Beller
2014-12-30 23:41 ` [PATCHv9 7/9] send-pack.c: add --atomic command line argument Stefan Beller
2014-12-30 23:41 ` [PATCHv9 8/9] push.c: add an --atomic argument Stefan Beller
2014-12-30 23:41 ` [PATCHv9 9/9] t5543-atomic-push.sh: add basic tests for atomic pushes Stefan Beller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150105202244.GL29365@google.com \
    --to=jrnieder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=mhagger@alum.mit.edu \
    --cc=pclouds@gmail.com \
    --cc=ronniesahlberg@gmail.com \
    --cc=sbeller@google.com \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.