Re: [PATCH v2 0/2] Allow disabling advice shown after merge conflicts
- From
Rubén Justo <rjusto@gmail.com>
- Date
- Mar 11, 2024, 20:58 UTC
- Message-ID
- <4e1abd12-d73f-41b2-a334-036be9093485@gmail.com>
- In-Reply-To
- <pull.1682.v2.git.1710100261.gitgitgadget@gmail.com>
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
[...]
Show 16 quoted lines
> ## 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.
Show 16 quoted lines
> ## 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.