Git Mailing List Archive on lore.kernel.org
 help / color / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Git List Mailing <git@vger.kernel.org>
Subject: Re: Avoiding 'master' nomenclature
Date: Wed, 29 Jul 2020 20:14:42 -0400
Message-ID: <20200730001442.GA2996059@coredump.intra.peff.net> (raw)
In-Reply-To: <xmqqlfj27x7q.fsf@gitster.c.googlers.com>

On Wed, Jul 29, 2020 at 03:50:01PM -0700, Junio C Hamano wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > The fast-export side lifted the "single branch is special"; we
> > didn't do something similar for "fmt-merge-msg".
> >
> >> So I think a path forward is more like:
> >>
> >>   1. Add a new config option to shorten fmt-merge-msg's output when the
> >>      destination branch matches it (and this should perhaps not even be
> >>      a single name, but a set of globs, which supports more workflows).
> >>      Call it merge.suppressDest or something.
> >>
> >>   2. Optionally a repository created with "git init" could copy its
> >>      init.defaultBranch into merge.suppressDest. And likewise a clone
> >>      might copy the remote HEAD into that variable. I'm not sure if that
> >>      is worth doing or not, but it would restore the original behavior
> >>      for the most part.
> >
> > Yeah, that sounds like a good plan.
> 
> A rough outline I did while waiting for today's integration builds
> to finish looks like this, which does not look _too_ bad.  We can
> replace the literal 'master' with the default branch name determined
> at runtime, but I am not sure if that is needed.

This looks like a good direction overall.

I'm on the fence on how magical to make the default. Having "master"
there gets Linus's case back where he wanted without having to configure
anything, which is probably reasonable. I'm not sure if people would
want their init.defaultBranch in addition / instead. Since it's a list
it's tempting to say that those could be added to the list even if the
user has specified some value, but I guess that makes things awkward if
you don't want them (I see you put in a way to clear the list, which is
good; I'm more talking about the fact that people would have to actually
remember to do so in their config).

Just a few comments on the patch itself:

>  fmt-merge-msg.c | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)

Tests and docs, obviously, but I know this is just a preview. :)

> +	} else if (!strcmp(key, "merge.suppressdest")) {
> +		if (!value)
> +			string_list_clear(&suppress_dest_patterns, 0);
> +		else
> +			string_list_append(&suppress_dest_patterns, value);
> +		suppress_dest_pattern_seen = 1;

I kind of hate the option name, despite being the one who suggested it.
But I don't have anything better. I do like naming it after the specific
action we plan to use it for, and not some "these are default branches"
list, which is too vague.

> @@ -451,7 +461,15 @@ static void fmt_merge_msg_title(struct strbuf *out,
>  			strbuf_addf(out, " of %s", srcs.items[i].string);
>  	}
>  
> -	strbuf_addf(out, " into %s\n", current_branch);
> +	for_each_string_list_item(item, &suppress_dest_patterns) {
> +		if (!wildmatch(item->string, current_branch, WM_PATHNAME)) {
> +			suppress_merge_dest = 1;
> +			break;
> +		}
> +	}

I think a list with globs should be plenty flexible. I really hope
nobody would need to include "foo*" but exclude "*bar" from that. If
they do they can write that patch later.

I think this will be matching branch names, not fully qualified refs.
Seems reasonable, but we should be sure to document that.

This loop might be nicer in a helper function with an early return, if
only to avoid extra local variables in fmt_merge_msg_title().

-Peff

  reply index

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-29 19:44 Linus Torvalds
2020-07-29 20:04 ` Junio C Hamano
2020-07-29 20:23   ` Linus Torvalds
2020-07-29 20:38     ` Jonathan Nieder
2020-07-29 20:46       ` Linus Torvalds
2020-07-29 20:56         ` Linus Torvalds
2020-07-30  8:17       ` lego_12239
2020-07-31  0:57         ` Jeff King
2020-07-31  8:19           ` Oleg
2020-07-29 20:40     ` Linus Torvalds
2020-07-29 20:58       ` Jeff King
2020-07-29 21:20         ` Linus Torvalds
2020-07-30  0:29           ` Jeff King
2020-07-30  0:44             ` Linus Torvalds
2020-07-30  0:52               ` Jeff King
2020-07-30  0:57                 ` Linus Torvalds
2020-07-31  0:44                   ` Jeff King
2020-07-29 21:25         ` Junio C Hamano
2020-07-29 22:50           ` Junio C Hamano
2020-07-30  0:14             ` Jeff King [this message]
2020-07-30  0:23               ` Linus Torvalds
2020-07-30 10:11                 ` Michal Suchánek
2020-07-30  0:31               ` Jeff King
2020-07-30  0:36             ` Junio C Hamano
2020-07-30 18:02               ` [PATCH v3 0/2] fmt-merge-msg: selectively suppress "into <branch>" Junio C Hamano
2020-07-30 18:02                 ` [PATCH v3 1/2] Revert "fmt-merge-msg: stop treating `master` specially" Junio C Hamano
2020-07-30 19:10                   ` Eric Sunshine
2020-07-30 19:40                     ` Junio C Hamano
2020-07-30 18:02                 ` [PATCH v3 2/2] fmt-merge-msg: allow merge destination to be omitted again Junio C Hamano
2020-07-31  0:42                 ` [PATCH v3 0/2] fmt-merge-msg: selectively suppress "into <branch>" Jeff King
2020-07-31  2:04                   ` Junio C Hamano
2020-07-31  2:22                     ` Jeff King
2020-07-31 20:03                       ` Taylor Blau
2020-07-31 20:12                         ` Junio C Hamano
2020-07-31 20:17                           ` Taylor Blau
2020-08-01  7:15                         ` Michal Suchánek
2020-08-10 11:53               ` Avoiding 'master' nomenclature Johannes Schindelin
2020-08-10 15:45                 ` Junio C Hamano
2020-08-11  2:39                   ` Johannes Schindelin
2020-08-12  0:30                     ` Junio C Hamano
2020-07-29 20:40     ` Junio C Hamano
2020-07-29 20:51       ` Linus Torvalds

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=20200730001442.GA2996059@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=torvalds@linux-foundation.org \
    /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

Git Mailing List Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/git/0 git/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 git git/ https://lore.kernel.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.git


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git