Re: [PATCH v2 0/9] Handle errors when setting configs

Jeff King [off-list ref] writes:

The end result is roughly the same, but it's a lot less churn, and it's
more likely for new callers to get it right, because they have to go the
extra mile to ignore the error. I say "roughly" because it treats cases
we missed as "die", whereas yours leaves them as "ignore". I find it
highly unlikely that any of them actually _want_ the ignore behavior,
though.

Yes, I like this approach better.  It admittedly is more risky in
that it would die if the conversion missed a case that should
ignore, but I suspect that such a breakage would be found rather
quickly (and the one that goes latent are the ones that do not
matter in practice because people would not encounter them).

I'm just pondering, though. I don't find the "or_die" variant bad at
all, so if you really prefer it, I don't mind.

Just to get a sense of what the reverse would look like, I worked up the
patch below (which compiles but does not link, as I did not actually
implement the "gently" form). Some error-checking call-sites are
converted to the "die" form, because that's essentially what happens
anyway (and I'd venture to say that the config code can provide a much
better error message).

This variant certainly looks nicer to me, for the reasons give above.

On Fri, Jan 29, 2016 at 10:55:52AM -0800, Junio C Hamano wrote:

Jeff King [off-list ref] writes:

quoted

The end result is roughly the same, but it's a lot less churn, and it's
more likely for new callers to get it right, because they have to go the
extra mile to ignore the error. I say "roughly" because it treats cases
we missed as "die", whereas yours leaves them as "ignore". I find it
highly unlikely that any of them actually _want_ the ignore behavior,
though.

Yes, I like this approach better.  It admittedly is more risky in
that it would die if the conversion missed a case that should
ignore, but I suspect that such a breakage would be found rather
quickly (and the one that goes latent are the ones that do not
matter in practice because people would not encounter them).

quoted

I'm just pondering, though. I don't find the "or_die" variant bad at
all, so if you really prefer it, I don't mind.

Just to get a sense of what the reverse would look like, I worked up the
patch below (which compiles but does not link, as I did not actually
implement the "gently" form). Some error-checking call-sites are
converted to the "die" form, because that's essentially what happens
anyway (and I'd venture to say that the config code can provide a much
better error message).

This variant certainly looks nicer to me, for the reasons give above.

Okay, thanks for your feedback. I'll create a new version next
week, then.

Patrick