Subject: Re: [PATCH 11/15] scalar: allow reconfiguring an existing enlistment

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Mon, 06 Sep 2021 03:01:03 +0200

On Fri, Sep 03 2021, Johannes Schindelin wrote:

> Hi Ævar,
>
> On Tue, 31 Aug 2021, Ævar Arnfjörð Bjarmason wrote:
>
>>
>> On Mon, Aug 30 2021, Johannes Schindelin via GitGitGadget wrote:
>>
>> > This comes in handy during Scalar upgrades, or when config settings were
>> > messed up by mistake.
>>
>> > [...]
>> >  		const char *key;
>> >  		const char *value;
>> > +		int overwrite_on_reconfigure;
>>
>> If you make this a "keep_on_reconfigure", then ...
>
> I do not think that this would be a better name, or that renaming this
> field would do anything except cause more work for me.

It would also result in more readable code, i.e. why add boilerplate ",
1" to a boolean field in this case if every single setting is set to
"1"? Doesn't it make more sense to invert the variable name & save on
the verbosity?

>>
>> >  	} config[] = {
>> > -		{ "am.keepCR", "true" },
>> > -		{ "core.FSCache", "true" },
>> > -		{ "core.multiPackIndex", "true" },
>> > -		{ "core.preloadIndex", "true" },
>> > +		/* Required */
>> > +		{ "am.keepCR", "true", 1 },
>> > +		{ "core.FSCache", "true", 1 },
>> > +		{ "core.multiPackIndex", "true", 1 },
>> > +		{ "core.preloadIndex", "true", 1 },
>>
>> You won't need the churn/boilerplate of adding "1" to everything here,
>> but can just change the initial patch to use designated initializers.
>>
>> That along with a throwaway macro like:
>>
>> #define SCALAR_CFG_TRUE(k) (.key = k, .value = "true")
>> #define SCALAR_CFG_FALSE(k) (.key = k, .value = "false")
>>
>> Might (or might not) make this even easier to eyeball...
>
> To me, it makes things less readable. There is an entire section with the
> header `/* Optional */` below, and I want this list to stay as readable as
> it is now.

Yeah, I think those macros are probably less readable too. I should have
phrased that as a "one could even...", but just the smaller change of
avoiding the ", 1" everywhere seems worthwhile.