Re: [PATCH v2 2/3] sequencer: make commit options more extensible

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 27, 2017, 00:55 UTC
Message-ID: <xmqqwpbb35e3.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <alpine.DEB.2.20.1703250100340.3767@virtualbox>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Show 6 quoted lines

>> Making "flags" unsigned was a correct change, but this is now wrong,
>> as "allow" is made unsigned by accident.
> ...
>
> Your patch looks good, you could do even better by reverting that move
> (IIRC it was at the end of the line, and it was set to 0 by default).

I do not think the variable needs to be initialized to anything (it is not looked at until it gets the result from allow_empty() call).

Anyway, the series is not yet in 'next', so you can replace it to the shape you would have made it into if you noticed that "allow" shouldn't be unsigned, while updating the log message to explain what the change is about (instead of only attempting to justify the past, which is not interesting in the context of understanding what this change is about).

As this thing is about fixing a regression visible to end users, I may get around to fixing things up myself, but I have other topics to attend to, so...