git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Damien Robert <damien.olivier.robert@gmail.com>,
	git@vger.kernel.org,
	Damien Robert <damien.olivier.robert+git@gmail.com>
Subject: Re: [PATCH v2 1/2] remote: drop "explicit" parameter from remote_ref_for_branch()
Date: Tue, 3 Mar 2020 13:11:42 -0800	[thread overview]
Message-ID: <20200303211142.GA36275@coredump.intra.peff.net> (raw)
In-Reply-To: <xmqqzhcx8gz8.fsf@gitster-ct.c.googlers.com>

On Tue, Mar 03, 2020 at 09:51:07AM -0800, Junio C Hamano wrote:

> > But unlike remote names, there's no default case for the remote branch
> > name.
> 
> Up to this point, it is well written and easy to read.  I think
> "there is no case where a default name for the remoate branch is
> used" would be even more easy to read.
> 
> In any case, if there is no case that default name, I understand
> that explicit is always set to 1?
> 
> > In any case where we don't set "explicit", we'd just an empty
> > string anyway.
> 
> Sorry, but I cannot parse this.  But earlier, you established that
> there is no case that a default is used, so is there any case where
> we don't set "explicit"?  I don't get it.

Maybe more clear:

For remote names, we will always return one of two things:

  - a remote name based on user config, in which case explicit=1

  - the default string "origin", in which case explicit=0

For remote branches, we will return either:

  - the remote branch name from config, in which case explicit=1

  - the empty string, in which case explicit=0

But nobody ever looks at that empty string. For the second case, we
could just as well return NULL. At which point we don't need an explicit
flag at all, as the caller can just check for NULL.

(written before reading what you wrote below)

> After reading the code through, I think "there's no default case"
> was what caused my confusion.
> 
>     But unlike remote names, there is no default name when the user
>     didn't configure one.  The only way the "explicit" parameter is
>     used by the caller is to use the value returned from the helper
>     when it is set, and use an empty string otherwise, ignoring the
>     returned value from the helper.
> 
>     Let's drop the "explicit" out-parameter, and return NULL when
>     the returned value from the helper should be ignored, to
>     simplify the function interface.

Yes, that looks fine to me.

-Peff

  reply	other threads:[~2020-03-03 21:11 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-28 17:24 [PATCH 1/1] remote.c: fix handling of push:remote_ref Damien Robert
2020-02-28 18:23 ` Jeff King
2020-03-01 22:05   ` Damien Robert
2020-03-02 13:32     ` Jeff King
2020-03-03 16:12       ` [PATCH v2 0/2] Damien Robert
2020-03-03 16:12         ` [PATCH v2 1/2] remote: drop "explicit" parameter from remote_ref_for_branch() Damien Robert
2020-03-03 17:51           ` Junio C Hamano
2020-03-03 21:11             ` Jeff King [this message]
2020-03-03 22:22               ` Junio C Hamano
2020-03-03 16:12         ` [PATCH v2 2/2] remote.c: fix handling of %(push:remoteref) Damien Robert
2020-03-03 16:29           ` Damien Robert
2020-03-03 18:29             ` Junio C Hamano
2020-03-03 18:21           ` Junio C Hamano
2020-03-03 22:24             ` Damien Robert
2020-03-03 22:48               ` Junio C Hamano
2020-03-12 16:45           ` [PATCH v3 1/1] " Damien Robert
2020-03-25 22:16             ` Damien Robert
2020-03-27 22:08               ` Junio C Hamano
2020-03-28 22:25                 ` Damien Robert
2020-03-28 13:15             ` Jeff King
2020-03-28 13:31               ` Jeff King
2020-04-16 15:12                 ` Damien Robert
2020-04-06 16:04               ` Damien Robert
2020-04-06 21:46                 ` Jeff King
2020-04-06 17:56             ` [RFC PATCH v4 0/2] %(push) and %(push:remoteref) bug fixes Damien Robert
2020-04-06 17:56               ` [PATCH v6 1/2] remote.c: fix %(push) for triangular workflows Damien Robert
2020-04-06 17:56               ` [PATCH v6 2/2] remote.c: fix handling of %(push:remoteref) Damien Robert
2020-04-16 15:03             ` [PATCH v8 1/1] " Damien Robert
2020-04-16 15:21               ` Damien Robert
2020-09-03 22:01                 ` Junio C Hamano
2020-09-11 21:43                   ` Damien Robert
2020-09-14 22:21                     ` Junio C Hamano
2020-03-03 16:16       ` [PATCH 1/1] remote.c: fix handling of push:remote_ref Damien Robert
2020-03-02 13:48     ` Jeff King
2020-03-03 16:25       ` Damien Robert

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20200303211142.GA36275@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=damien.olivier.robert+git@gmail.com \
    --cc=damien.olivier.robert@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --subject='Re: [PATCH v2 1/2] remote: drop "explicit" parameter from remote_ref_for_branch()' \
    /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

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).