Re: [PATCH v4 2/3] replay: make atomic ref updates the default behavior

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 22, 2025, 21:19 UTC
Message-ID: <xmqq7bwmy6r6.fsf@gitster.g>
In-Reply-To: <20251022185045.29256-3-siddharthasthana31@gmail.com>

Siddharth Asthana <siddharthasthana31@gmail.com> writes:

Show 13 quoted lines

> 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.