Re: [PATCH] gitweb: fixes to gitweb feature check code

[Junio C Hamano <gitster@pobox.com>]

Jakub Narebski <jnareb@gmail.com> writes:

> What I like about having all this, i.e. fix, futureproof and style
> correction in one single patch is the fact that fix doesn't introduce
> strange looking (gitweb_check_feature('bool_feat'))[0]... well, except
> encapsulated in a subroutine.
>
> From all possible splits of this feature into series of up to three
> patches I think I like the one with pure subroutine rename from *check*
> to *get* least...

Well, I have to say that you have a strange taste, sense of priorities,
and perhaps aversion to logical progression.  Let's explain one more
time.

The case we had at hand was that a callee has a less-than-ideal calling
convention that has caused a few bugs by callers because they did not
understand the calling convention.  You can argue it is not entirely
caller's fault that they failed to follow the calling convention, but the
fact remains that there are bugs taken as a whole.

First we fix the callers, because existing bugs get highest priority.
This is a pure bugfix patch that could even go to maintenance "bugfix
only" branch.

Then we fix the calling convention because we all know that the calling
convention was less-than-ideal.  A large part of the reason the calling
convention was confusing was because the wording "check" implied it was a
boolean function.  Logically, s/check/get/ would be a major part of fixing
that.

After calling convention is enhanced by a new function that lets callers
"check" via a boolean function, we can have them use that, which makes
them easier to read.

But remember that it is the order I wanted the patches to be presented for
review.  After people involved in review agree that the result is good, I
do not have any problem in squashing the three steps into a single patch
for things like this after the end result is verified to be good (which we
did).