Re: [PATCH/RFC] parse-options.c: make OPTION__COUNTUP consider negative values

On Sun, Mar 20, 2016 at 9:40 AM, Jeff King [off-list ref] wrote:

quoted

quoted

We are also changing semantics without changing the interface, which
means any topics in flight (that you _cannot_ review, because you have
not seen them yet) may be subtly broken. To me that is not an absolute
deal-breaker, but something to weigh against the utility of the change.

As I am new here, I don't really know how to go about with this. Could
you describe in a little detail so that I can work on it?

A more canonical example is changing a function return value. Imagine I
have a function which returns "1" for success and "0" for error, and I
want to change it to return "0" for success and "-1" for error. If I do
so and update each caller, then merging with a branch that has a new
caller will not result in any conflicts (there is no textual link
between the callers and the function), but the result will be subtly
broken (the new caller will get the error-check wrong).

So we generally try to find some way that the compiler will notice the
breakage. E.g., if the function changes name when the return value
semantics change, or if it gains a new argument at the same time, then
the compiler will notice and complain. We still have to fix it up during
the merge, of course, but it's easy to spot.

Likewise here. If you change the semantics of OPT_COUNTUP(), then any
branch which introduces a new use of "int foo = -1" and expects the old
semantics will be subtly broken.

The obvious fix would be to switch the name (to OPT_COUNTUP_DEFAULT() or
something). But that's a bit painful, as almost nobody uses COUNTUP
directly, so we'd need OPT__VERBOSE_DEFAULT().

This is quite new to me. It took me some time to digest it.

Which in the end is the same as ignoring the problem in the first place,
but there is a big difference between not thinking about the problem,
and thinking about it and deciding it's not a problem. :)

True That! ;)

Regards,
Pranit Bauva