From: Jeff King <peff@peff.net>
Date: Fri, 04 Dec 2009 10:20:39 GMT
Subject: Re: [PATCH resend 3/3] transport.c::transport_push(): make ref status affect return value
Message-ID: <20091204102039.GB27495@coredump.intra.peff.net>
In-Reply-To: <20091204145842.57c3c51c.rctay89@gmail.com>

On Fri, Dec 04, 2009 at 02:58:42PM +0800, Tay Ray Chuan wrote:

> -		if (!quiet || push_had_errors(remote_refs))
> -			print_push_status(transport->url, remote_refs,
> -					verbose | porcelain, porcelain,
> -					nonfastforward);
> +		if (!quiet)
> +			if (push_had_errors(remote_refs)) {
> +				ret = -1;
> +				print_push_status(transport->url, remote_refs,
> +						verbose | porcelain, porcelain,
> +						nonfastforward);
> +			}

Er, this doesn't seem right. It will no longer print anything under
non-quiet mode unless there are errors, and it will only set a return
value in non-quiet mode.

I think you want:

  if (push_had_errors(remote_refs)) {
          ret = -1;
          if (!quiet)
                  print_push_status(...)
  }
  else if (!quiet)
          print_push_status(...)

Actually, it may simply be more readable by storing the error result:

  int err = push_had_errors(remote_refs);
  if (err)
    ret = -1;
  if (!quiet || err)
    print_push_status(...);

-Peff