Re: [PATCH 3/3] gitweb: make gitweb_check_feature a boolean wrapper

["Giuseppe Bilotta" <giuseppe.bilotta@gmail.com>]

On Sun, Nov 30, 2008 at 1:23 AM, Junio C Hamano <gitster@pobox.com> wrote:
> "Giuseppe Bilotta" <giuseppe.bilotta@gmail.com> writes:
>
>> On Sat, Nov 29, 2008 at 10:16 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>>  * Again, this is to demonstrate how I would have liked to see your
>>>   patches as a reviewable series.  Notice how this naturally justifies
>>>   the dropping of parentheses in many lines that begin with "my", and
>>>   makes it easier to review?  You can review the patch easily by knowing
>>>   that callers that have s/get/check/ are now safe to use scalar context.
>>
>> Yes, I get the point: do less things under the hood. I also still
>> think that three patches with code going ping-pong are way too much,
>> though 8-P
>>
>> For what it's worth, you get my Ack: to your patchset 8-)
>
> Well, I am not interested in taking credits for this series.  I am very
> much interested in reducing my future workload by showing people how an
> easily reviewable series should look like.
>
> So if you still think it is "way too much", I did not succeed what I tried
> to do X-<.

At least as far as this patch goes, from my point of view, the
'clueless/careless' usages of gitweb_check_feature are the
conceptually (although not code-wise) correct ones, so they shouldn't
be the ones touched by the patch, considering that in addition to the
futureproofing, (implicit) fixing those usages is one of the main
points of the patch itself.

OTOH, I _do_ get the point about ease of review. But for example (and
for future reference for myself): would you say that it would have
been enough to have a cleaner commit message explicitly mentioning the
fact that formerly incorrect usages of the gitweb_check_feature() were
left intact because they were now correct?

Something like this http://git.oblomov.eu/git/patch/gitweb/check-feature-v3

-- 
Giuseppe "Oblomov" Bilotta