Re: [PATCH 11/15] scalar: allow reconfiguring an existing enlistment
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Sep 6, 2021, 01:01 UTC
- Message-ID
- <87bl56plbi.fsf@evledraar.gmail.com>
- In-Reply-To
- <nycvar.QRO.7.76.6.2109031750291.55@tvgsbejvaqbjf.bet>
On Fri, Sep 03 2021, Johannes Schindelin wrote:
Show 19 quoted lines
> 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?
Show 25 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.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.