All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Anders Kaseorg <andersk@mit.edu>
Cc: git@vger.kernel.org,
	"Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
	"Jeff King" <peff@peff.net>,
	"Andreas Heiduk" <andreas.heiduk@mathema.de>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: Re: [PATCH v6 1/8] fetch: lowercase error messages
Date: Mon, 15 Nov 2021 21:19:08 -0800	[thread overview]
Message-ID: <xmqqczn0d6er.fsf@gitster.g> (raw)
In-Reply-To: 20211113033358.2179376-2-andersk@mit.edu

Anders Kaseorg <andersk@mit.edu> writes:

> Documentation/CodingGuidelines says “do not end error messages with a
> full stop” and “do not capitalize the first word”.  Reviewers requested
> updating the existing messages to comply with these guidelines prior to
> the following patches.

Thanks.  Whether reviewers requested or you thought of it on your
own, separating such a preliminary clean-up into its own patch would
be a good idea, especially if the later patches need to update (some
of) them.


> @@ -1062,13 +1062,13 @@ static void close_fetch_head(struct fetch_head *fetch_head)
>  }
>  
>  static const char warn_show_forced_updates[] =
> -N_("Fetch normally indicates which branches had a forced update,\n"
> -   "but that check has been disabled. To re-enable, use '--show-forced-updates'\n"
> -   "flag or run 'git config fetch.showForcedUpdates true'.");
> +N_("fetch normally indicates which branches had a forced update,\n"
> +   "but that check has been disabled; to re-enable, use '--show-forced-updates'\n"
> +   "flag or run 'git config fetch.showForcedUpdates true'");
>  static const char warn_time_show_forced_updates[] =
> -N_("It took %.2f seconds to check forced updates. You can use\n"
> +N_("it took %.2f seconds to check forced updates; you can use\n"
>     "'--no-show-forced-updates' or run 'git config fetch.showForcedUpdates false'\n"
> -   " to avoid this check.\n");
> +   " to avoid this check\n");

The two guidelines cited in the proposed log message are primarily
to prefer

    fatal: unrecognized argument: --no-such-option

over

    fatal: Unrecognized argument: --no-such-option.

and does not say much what to do to a multi-sentence message.  In
this part (and other parts) of the patch, I can see that you thought
this one through when preparing this patch.  I very much appreciate
it.

The approach chosen (consistently) in this patch is to

 (1) turn them into a (semi) single sentence, concatenated with ';'

 (2) as a side effect of not being a free-standing sentence anymore,
     the second and subsequent sentences in the original, that are
     now just pieces in a single sentence separated with ';', do not
     get capitalized, and

 (3) the sentence as a whole lacks the full-stop, just like a single
     sentence message.

I think we are fine with these rules, especially given that these
multi-sentence messages are not the main part of this topic touches
and are not the primary focus of this topic anyway.  

I am highlighting this part of the change, just in case others think
of a better set of rules to follow.  Existing multi-sentence messages
follow different ad-hoc patterns, it seems (e.g. "git show 00000000").

Thanks.


  reply	other threads:[~2021-11-16  5:19 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-13  3:33 [PATCH v6 0/8] protect branches checked out in all worktrees Anders Kaseorg
2021-11-13  3:33 ` [PATCH v6 1/8] fetch: lowercase error messages Anders Kaseorg
2021-11-16  5:19   ` Junio C Hamano [this message]
2021-11-16  7:10     ` Anders Kaseorg
2021-11-17  8:09       ` Junio C Hamano
2021-11-22  1:14   ` Jiang Xin
2021-11-13  3:33 ` [PATCH v6 2/8] receive-pack: " Anders Kaseorg
2021-11-18  4:53   ` Junio C Hamano
2021-11-13  3:33 ` [PATCH v6 3/8] branch: " Anders Kaseorg
2021-11-13  3:33 ` [PATCH v6 4/8] worktree: simplify find_shared_symref() memory ownership model Anders Kaseorg
2021-11-16  5:39   ` Junio C Hamano
2021-11-22 12:45   ` Johannes Schindelin
2021-11-13  3:33 ` [PATCH v6 5/8] fetch: protect branches checked out in all worktrees Anders Kaseorg
2021-11-16  5:49   ` Junio C Hamano
2021-11-16  6:44     ` Anders Kaseorg
2021-11-22 13:13   ` Johannes Schindelin
2021-11-13  3:33 ` [PATCH v6 6/8] receive-pack: clean dead code from update_worktree() Anders Kaseorg
2021-11-16  5:49   ` Junio C Hamano
2021-11-13  3:33 ` [PATCH v6 7/8] receive-pack: protect current branch for bare repository worktree Anders Kaseorg
2021-11-13  3:33 ` [PATCH v6 8/8] branch: protect branches checked out in all worktrees Anders Kaseorg
2021-11-22 13:21 ` [PATCH v6 0/8] " Johannes Schindelin

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=xmqqczn0d6er.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=andersk@mit.edu \
    --cc=andreas.heiduk@mathema.de \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    /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.