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

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Dec 22, 2014, 22:46 UTC
Message-ID: <alpine.DEB.1.00.1412222344250.21312@s15462909.onlinehome-server.info>
In-Reply-To: <xmqq38878gao.fsf@gitster.dls.corp.google.com>

Hi Junio,

On Mon, 22 Dec 2014, Junio C Hamano wrote:

Show 13 quoted lines

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > Or do you want the error messages to also use camelCased IDs, i.e.
> >
> > 	warning in tag $tag: missingTaggerEntry: invalid format ...
> >
> > instead of
> >
> > 	warning in tag $tag: missing-tagger-entry: invalid format ...
> >
> > ? It is easy to do, but looks slightly uglier to this developer's eyes...
> 
> Do you really need to know what I think?

Well, but yes ;-)

> Can I say "The latter is probably slightly better, but both look ugly to
> me"?

Of course you can say that! ;-) The problem these ugly messages try to solve is to give the user a hint which setting to change if they want to override the default behavior, though...

Show 10 quoted lines

> If we want a human readable message
> 
>     "warning: tag object lacks tagger field '$tag'"
> 
> would be my preference.
> 
> But I personally think it may not be necessary to give a pretty
> message that later can go through l10n here.  If we take that
> position, it is just a machine-readble error token, so I'd say
> whichever way you find more readable is OK.

Okay, I will leave it as-is, then.

Show 22 quoted lines

> >> Should these be tied to receive-pack ones in any way?  E.g. if you
> >> set fsck.missingEmail to ignore, you do not have to do the same for
> >> receive and accept a push with the specific error turned off?
> >> 
> >> Not a rhetorical question.  I can see it argued both ways.  The
> >> justification to defend the position of not tying these two I would
> >> have is so that I can be more strict to newer breakages (i.e. not
> >> accepting a push that introduce a new breakage by not ignoring with
> >> receive.fsck.*) while allowing breakages that are already present.
> >> The justification for the opposite position is to make it more
> >> convenient to write a consistent policy.  Whichever way is chosen,
> >> we would want to see the reason left in the log message so that
> >> people do not have to wonder what the original motivation was when
> >> they decide if it is a good idea to change this part of the code.
> >
> > Hmm. I really tried very hard to separate the fsck.* from the receive.*
> > settings because the two code paths already behave differently:...
> >
> > If you agree, I would rephrase this line of argument and add it to the
> > commit message. Do you agree?
> 
> Yeah, that reasoning sounds sensible.

I added this paragraph:

    In the same spirit that `git receive-pack`'s usage of the fsck machinery
    differs from `git fsck`'s – some of the non-fatal warnings in `git fsck`
    are fatal with `git receive-pack` when receive.fsckObjects = true, for
    example – we strictly separate the fsck.* from the receive.fsck.*
    settings.

Ciao, Dscho