On Mon, Dec 11, 2023 at 07:52:28PM -0500, Jeff King wrote: > On Thu, Dec 07, 2023 at 09:14:36AM +0100, Patrick Steinhardt wrote: > > > Thanks for working on this topic! I've left a couple of comments, most > > of which are about whether we should retain previous behaviour where we > > generate a warning instead of raising an error for unknown values. > > Thanks for taking a look. I see what you're saying about the warnings, > but IMHO it's not worth the extra complexity. Returning early means the > existing code can proceed without worrying about NULLs. Though I suppose > we could have a "warn_error_nonbool()" which issues a warning and > returns 0. > > Still, the "return config_error_nonbool()" pattern is pretty > well-established and used for most options. I would go so far as to say > the ones that warn for invalid values are the odd ones out, and probably > should be returning errors as well (though doing so now may not be worth > the trouble and risk of annoyance). > > And certainly there should be no regressions in this series; every case > is currently a segfault, so returning an error is a strict improvement. > I'd just as soon stay strict there, as it's easier to loosen later if we > choose than to tighten. Fair enough, I'm perfectly fine with this reasoning. Thanks! Patrick
Attachment:
signature.asc
Description: PGP signature
