From: Junio C Hamano <gitster@pobox.com>
Date: Wed, 22 Oct 2025 21:19:09 GMT
Subject: Re: [PATCH v4 2/3] replay: make atomic ref updates the default behavior
Message-ID: <xmqq7bwmy6r6.fsf@gitster.g>
In-Reply-To: <20251022185045.29256-3-siddharthasthana31@gmail.com>

Siddharth Asthana <siddharthasthana31@gmail.com> writes:

> diff --git a/builtin/replay.c b/builtin/replay.c
> index b64fc72063..1246add636 100644
> --- a/builtin/replay.c
> +++ b/builtin/replay.c
> @@ -20,6 +20,11 @@
>  #include <oidset.h>
>  #include <tree.h>
>  
> +enum ref_action_mode {
> +	REF_ACTION_UPDATE,
> +	REF_ACTION_PRINT
> +};
> +

We allow and encourage the last item in enum definition to have
trailing comma, i.e.

        enum ref_action_mode {
                REF_ACTION_UPDATE,
                REF_ACTION_PRINT,
        };

unless the last one is somehow special and we are not supposed to
add any new item after that (e.g., a sentinel REF_ACTION_MAX that is
supposed to give the upper limit of the values).  That way, future
developers can add new items with minimum patch noise.

> @@ -434,10 +491,15 @@ int cmd_replay(int argc,
> ...
> +					ret = error(_("failed to update ref %s: %s"),
> +						    decoration->name, transaction_err.buf);

Hmph, don't we want to use '%s' when reporting the ->name thing?
Documentation/CodingGuidelines has this:

   Error Messages

    - Do not end a single-sentence error message with a full stop.

    - Do not capitalize the first word, only because it is the first word
      in the message ("unable to open '%s'", not "Unable to open '%s'").  But
      "SHA-3 not supported" is fine, because the reason the first word is
      capitalized is not because it is at the beginning of the sentence,
      but because the word would be spelled in capital letters even when
      it appeared in the middle of the sentence.

    - Say what the error is first ("cannot open '%s'", not "%s: cannot open").

    - Enclose the subject of an error inside a pair of single quotes,
      e.g. `die(_("unable to open '%s'"), path)`.

    - Unless there is a compelling reason not to, error messages from
      porcelain commands should be marked for translation, e.g.
      `die(_("bad revision %s"), revision)`.

    - Error messages from the plumbing commands are sometimes meant for
      machine consumption and should not be marked for translation,
      e.g., `die("bad revision %s", revision)`.

    - BUG("message") are for communicating the specific error to developers,
      thus should not be translated.