Re: [PATCH v3] advice: use global config for default branch name

[Vsevolod Myalitsin <ub4nal@mail.ru>]

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