Subject: Re: [PATCH v6 2/8] receive-pack: lowercase error messages

From: Junio C Hamano <[email-masked]>
Date: Wed, 17 Nov 2021 20:53:38 -0800

Anders Kaseorg <[email-masked]> 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.

The same comment applies to this part.  Reviewers may make
suggestion to help polish the topic, but it ultimately is the
author's achievement.  More importantly, we write our log messages
to help future developers to learn from.  The fact somebody asked
some changes made is much less important than the reason why such
changes are desirable, so that they can craft their future topics
following the same pattern.

    Documentation... says X and Y.  Clean up existing messages, some
    of which we will be touching in later steps in the series, that
    deviate from these rules in this file, as a preparation for the
    main part of the topic.

may convey the intention better, I would think.

The patch text looks good.  Thanks.

> Signed-off-by: Anders Kaseorg <[email-masked]>
> ---
>  builtin/receive-pack.c          | 6 +++---
>  t/t5504-fetch-receive-strict.sh | 2 +-
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> index 2d1f97e1ca..a82b60f387 100644
> --- a/builtin/receive-pack.c
> +++ b/builtin/receive-pack.c
> @@ -170,7 +170,7 @@ static int receive_pack_config(const char *var, const char *value, void *cb)
>  			strbuf_addf(&fsck_msg_types, "%c%s=%s",
>  				fsck_msg_types.len ? ',' : '=', var, value);
>  		else
> -			warning("Skipping unknown msg id '%s'", var);
> +			warning("skipping unknown msg id '%s'", var);
>  		return 0;
>  	}
>  
> @@ -1584,9 +1584,9 @@ static const char *update(struct command *cmd, struct shallow_info *si)
>  		if (!parse_object(the_repository, old_oid)) {
>  			old_oid = NULL;
>  			if (ref_exists(name)) {
> -				rp_warning("Allowing deletion of corrupt ref.");
> +				rp_warning("allowing deletion of corrupt ref");
>  			} else {
> -				rp_warning("Deleting a non-existent ref.");
> +				rp_warning("deleting a non-existent ref");
>  				cmd->did_not_exist = 1;
>  			}
>  		}
> diff --git a/t/t5504-fetch-receive-strict.sh b/t/t5504-fetch-receive-strict.sh
> index 6e5a9c20e7..b0b795aca9 100755
> --- a/t/t5504-fetch-receive-strict.sh
> +++ b/t/t5504-fetch-receive-strict.sh
> @@ -292,7 +292,7 @@ test_expect_success 'push with receive.fsck.missingEmail=warn' '
>  		receive.fsck.missingEmail warn &&
>  	git push --porcelain dst bogus >act 2>&1 &&
>  	grep "missingEmail" act &&
> -	test_i18ngrep "Skipping unknown msg id.*whatever" act &&
> +	test_i18ngrep "skipping unknown msg id.*whatever" act &&
>  	git --git-dir=dst/.git branch -D bogus &&
>  	git --git-dir=dst/.git config --add \
>  		receive.fsck.missingEmail ignore &&