Subject: Re: [PATCH 16/18] fsck: support demoting errors to warnings

From: Junio C Hamano <[email-masked]>
Date: Tue, 23 Dec 2014 09:14:54 -0800

Johannes Schindelin <[email-masked]> writes:

> The parser I wrote actually accepts both versions, allowing me to skip the
> tedious step to convert the camelCased config setting into a
> lower-case-dashed version to pass to `index-pack` or `unpack-objects`,
> only to be parsed by the same parser as `fsck` would use directly.
>
> So I am rather happy with the fact that the parser handles both camelCased
> and lower-case-dashed versions.

That is myopic view of the world that ignores maintainability and
teachability, doing disservice to our user base.

What message does it send to an unsuspecting new user that
fsck.random-error is silently accepted (because we will never
document it) as an alias for fsck.randomError, while most of the
configuration variables will not take such an alias?

>> I suspect that it would be much better if the configuration variables
>> were organized the other way around, e.g.
>> 
>> 	$ git config fsck.warn missingTagger,someOtherKindOfError
>
> I had something similar in an earlier version of my patch series, but it
> was shot down rightfully: if you want to allow inheriting defaults from
> $HOME/.gitconfig, you have to configure the severity levels individually.

Hmmm.  What's wrong with "fsck.warn -missingTagger" that overrides
the earlier one, or even "fsck.info missingTagger" after having
"fsck.warn other,missingTagger,yetanother", with the usual "last one
wins" rule?

Whoever shot it down "rightfully" is wrong here, I would think.

>> But the proposed organization to use one variable per questionable
>> event type (as opposed to one variable per severity level) would
>> lead to a one-shot override of this form, e.g.
>> 
>> 	$ git fsck --missing-tagger=warn --some-other-kind-of-error=warn
>> 
>> which I think is insane to require us to support unbound number of
>> dashed options.
>
> The intended use case is actually *not* the command-line, but the config
> file, in particular allowing /etc/gitconfig, $HOME/.gitconfig *and*
> .git/config to customize the settings.

But we do need to worry about one-shot override from the command
line.  A configuration that sticks without a way to override is a
no-no.

>> Or are you saying that we allow "git config core.file-mode true"
>> from the command line to set core.fileMode configuration?
>
> I do not understand this reference.

I was puzzled by your "command line" and wondering if you meant
"from the command line, aVariable can be spelled a-variable".

> I did not suggest to change `git
> config`, did I? If I did, I apologize; it was definitely *not* my
> intention to change long-standing customs.

Then fsck.missing-tagger is definitely out.  Long standing customs
is that a multi-word token at the first and the last level is not
dashed-multi-word.