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

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Sep 3, 2021, 15:53 UTC
Message-ID: <nycvar.QRO.7.76.6.2109031750291.55@tvgsbejvaqbjf.bet>
In-Reply-To: <874kb6xbf9.fsf@evledraar.gmail.com>

Hi Ævar,

On Tue, 31 Aug 2021, Ævar Arnfjörð Bjarmason wrote:

Show 12 quoted lines

>
> 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.

Show 21 quoted lines

>
> >  	} 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.

Ciao, Dscho