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

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 2, 2008, 21:55 UTC
Message-ID: <7v4p1mp7hx.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <200812020253.09430.jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> writes:

Show 8 quoted lines

> 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).