Subject: Re: [PATCH v2 0/2] Allow disabling advice shown after merge conflicts

From: Rubén Justo <[email-masked]>
Date: Mon, 11 Mar 2024 21:58:52 +0100

On Sun, Mar 10, 2024 at 07:50:59PM +0000, Philippe Blain via GitGitGadget wrote:

> Range-diff vs v1:
> 
>  1:  e929d3381cf ! 1:  a2ce6fd24c2 sequencer: allow disabling conflict advice

[...]

>        ## advice.c ##
>       @@ advice.c: static struct {
>      - 	[ADVICE_RESET_NO_REFRESH_WARNING]		= { "resetNoRefresh" },
>      - 	[ADVICE_RESOLVE_CONFLICT]			= { "resolveConflict" },
>      - 	[ADVICE_RM_HINTS]				= { "rmHints" },
>      -+	[ADVICE_SEQUENCER_CONFLICT]                     = { "sequencerConflict" },
>      - 	[ADVICE_SEQUENCER_IN_USE]			= { "sequencerInUse" },
>      - 	[ADVICE_SET_UPSTREAM_FAILURE]			= { "setUpstreamFailure" },
>      - 	[ADVICE_SKIPPED_CHERRY_PICKS]			= { "skippedCherryPicks" },
>      + 	[ADVICE_GRAFT_FILE_DEPRECATED]			= { "graftFileDeprecated" },
>      + 	[ADVICE_IGNORED_HOOK]				= { "ignoredHook" },
>      + 	[ADVICE_IMPLICIT_IDENTITY]			= { "implicitIdentity" },
>      ++	[ADVICE_MERGE_CONFLICT]				= { "mergeConflict" },
>      + 	[ADVICE_NESTED_TAG]				= { "nestedTag" },
>      + 	[ADVICE_OBJECT_NAME_WARNING]			= { "objectNameWarning" },
>      + 	[ADVICE_PUSH_ALREADY_EXISTS]			= { "pushAlreadyExists" },

You rename ADVICE_SEQUENCER_CONFLICT to ADVICE_MERGE_CONFLICT and place
the new name in the correct position, alphabetically.  Nice.

>        ## advice.h ##
>       @@ advice.h: enum advice_type {
>      - 	ADVICE_RESOLVE_CONFLICT,
>      - 	ADVICE_RM_HINTS,
>      - 	ADVICE_SEQUENCER_IN_USE,
>      -+	ADVICE_SEQUENCER_CONFLICT,
>      - 	ADVICE_SET_UPSTREAM_FAILURE,
>      - 	ADVICE_SKIPPED_CHERRY_PICKS,
>      - 	ADVICE_STATUS_AHEAD_BEHIND_WARNING,
>      + 	ADVICE_IGNORED_HOOK,
>      + 	ADVICE_IMPLICIT_IDENTITY,
>      + 	ADVICE_NESTED_TAG,
>      ++	ADVICE_MERGE_CONFLICT,
>      + 	ADVICE_OBJECT_NAME_WARNING,
>      + 	ADVICE_PUSH_ALREADY_EXISTS,
>      + 	ADVICE_PUSH_FETCH_FIRST,

Here, I assume you're trying to place the new name correctly too.
However, I see that it's in the wrong place.  It initially caught my
attention, but then I realize that the list is not sorted.  So it's
understandable.

Maybe you want to sort the list as a preparatory patch in this series
and so we'll avoid this kind of mistakes.

Of course, this does not deserve a reroll.  We can do it in a future
series when the dust settles.