git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: Tao Klerks via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org, Glen Choo <chooglen@google.com>,
	Tao Klerks <tao@klerks.biz>
Subject: Re: [PATCH v5] tracking branches: add advice to ambiguous refspec error
Date: Wed, 30 Mar 2022 13:37:53 -0700	[thread overview]
Message-ID: <xmqqmth76use.fsf@gitster.g> (raw)
In-Reply-To: <220330.864k3fpo32.gmgdl@evledraar.gmail.com> ("Ævar Arnfjörð Bjarmason"'s message of "Wed, 30 Mar 2022 15:19:47 +0200")

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> On Wed, Mar 30 2022, Tao Klerks via GitGitGadget wrote:
>
>> From: Tao Klerks <tao@klerks.biz>
>
>> +		case 2:
>> +			// there are at least two remotes; backfill the first one
>
> Nit: I think it's been Junio's preference not to introduce C99 comments,

I often mention the rule to new contributors, simply because it has
been that way in our code base, regardless of what my personal
preference might be, and sticking to the style will be more
consistent.  It's not like I am forcing my personal preference on
developers.  Do not mislead new people into thinking so.  It is
especially irritating to see ...

> despite other C99 features now being used (and I think it should work in
> practice as far as portability goes, see
> https://lore.kernel.org/git/87wnmwpwyf.fsf@evledraar.gmail.com/)

... a mention like this, when you know that it is not about
portability but is about consistency, and also you know I've
mentioned more than once on the list if we want to loosen some
written CodingGuidelines rules, especially those that tools do not
necessarily catch.

>> +	if (tracking.matches > 1) {
>> +		int status = die_message(_("not tracking: ambiguous information for ref %s"),
>> +					    orig_ref);
>
> This isn't per-se new, but I wonder if while we're at it we shold just
> quote '%s' here, which we'd usually do. I.e. this message isn't new, but
> referring again to "ref %s" (and not "ref '%s'") below is.

Good.

>> +				 "To support setting up tracking branches, ensure that\n"
>> +				 "different remotes' fetch refspecs map into different\n"
>> +				 "tracking namespaces."),
>> +			       orig_ref,
>> +			       remotes_advice.buf
>> +			       );
>
> Nit: The usual style for multi-line arguments is to "fill" lines until
> you're at 79 characters, so these last three lines (including the ");")
> can all go on the "tracking namespaces" line (until they're at 79, then
> wrap)>

I didn't know about the magic "79" number.  It makes the resulting
source code extremely hard to read, though, while making it easier
to grep for specific messages.


  parent reply	other threads:[~2022-03-30 20:38 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-21 10:23 [PATCH] tracking branches: add advice to ambiguous refspec error Tao Klerks via GitGitGadget
2022-03-21 14:11 ` Ævar Arnfjörð Bjarmason
2022-03-22  9:09   ` Tao Klerks
2022-03-22  9:18 ` [PATCH v2] RFC: " Tao Klerks via GitGitGadget
2022-03-22 10:04   ` Ævar Arnfjörð Bjarmason
2022-03-28  6:51   ` [PATCH v3] " Tao Klerks via GitGitGadget
2022-03-28 16:23     ` Junio C Hamano
2022-03-28 17:12       ` Glen Choo
2022-03-28 17:23         ` Junio C Hamano
2022-03-28 18:02           ` Tao Klerks
2022-03-28 18:50             ` Ævar Arnfjörð Bjarmason
2022-03-28 20:32               ` Junio C Hamano
2022-03-28 20:27             ` Junio C Hamano
2022-03-29 11:26               ` Tao Klerks
2022-03-29 11:26     ` [PATCH v4] " Tao Klerks via GitGitGadget
2022-03-29 11:31       ` Tao Klerks
2022-03-29 15:49       ` Junio C Hamano
2022-03-30  4:17         ` Tao Klerks
2022-03-30  7:20       ` [PATCH v5] " Tao Klerks via GitGitGadget
2022-03-30 13:19         ` Ævar Arnfjörð Bjarmason
2022-03-30 14:23           ` Tao Klerks
2022-03-30 15:18             ` Tao Klerks
2022-03-30 17:14               ` Ævar Arnfjörð Bjarmason
2022-03-30 20:37           ` Junio C Hamano [this message]
2022-03-30 21:09             ` Ævar Arnfjörð Bjarmason
2022-03-30 22:07               ` Junio C Hamano
2022-03-30 22:51                 ` Ævar Arnfjörð Bjarmason
2022-03-31 16:01         ` [PATCH v6] " Tao Klerks via GitGitGadget
2022-03-31 19:32           ` Junio C Hamano
2022-03-31 23:57             ` Glen Choo
2022-04-01  4:30               ` Tao Klerks
2022-04-01 16:41                 ` Glen Choo
2022-03-31 19:33           ` Junio C Hamano
2022-04-01  6:05           ` [PATCH v7] " Tao Klerks via GitGitGadget
2022-04-01 16:53             ` Glen Choo
2022-04-01 19:57               ` Junio C Hamano

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=xmqqmth76use.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=avarab@gmail.com \
    --cc=chooglen@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=tao@klerks.biz \
    /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.
Code repositories for project(s) associated with this public 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).