Re: [PATCH v3] advice: use global config for default branch name
- From
- Vsevolod Myalitsin <ub4nal@mail.ru>
- Date
- Sep 9, 2026, 21:22 UTC
- Message-ID
- <20260909212214.94151-1-ub4nal@mail.ru>
- In-Reply-To
- <20260909202718.GA183838@coredump.intra.peff.net>
Hi Peff,
> Translators will need to update their message translations, and I wonder > if seeing this "set%s" in isolation might be confusing.
I agree that using a single placeholder for the whole command is clearer for translators. I'll use this approach.
> But really it is an enum, and if we are going to use "==" we should > probably spell out the whole name rather than 0, like:
if (setting && setting->level == ADVICE_LEVEL_NONE)
I simply forgot to include this change in the patch. I'll change it to use ADVICE_LEVEL_NONE.
> I had somehow hoped we could reuse the existing CONFIG_SCOPE enum > without having to redeclare it ourselves.
One concern about reusing enum config_scope: since CONFIG_SCOPE_UNKNOWN is 0, all existing advice_setting entries without an explicitly specified scope_hint would default to CONFIG_SCOPE_UNKNOWN rather than CONFIG_SCOPE_LOCAL.
I believe this is incorrect, since the existing behavior is local scope by default. However, if you consider CONFIG_SCOPE_UNKNOWN appropriate here and it satisfies the intended requirements, I have no objection to using the existing enum.
Thanks, Vsevolod