Re: [PATCH v3] gitweb: Add an option for adding more branch refs

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 2, 2013, 18:18 UTC
Message-ID: <xmqq61r7rua0.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1385985997.2054.27.camel@localhost.localdomain>

Krzesimir Nowak <krzesimir@endocode.com> writes:

Show 18 quoted lines

> On Mon, 2013-12-02 at 01:21 +0100, Jakub Narębski wrote:
>> On Thu, Nov 28, 2013 at 12:44 PM, Krzesimir Nowak
>> <krzesimir@endocode.com> wrote:
>> 
>> > Allow @additional_branch_refs configuration variable to tell gitweb to
>> > show refs from additional hierarchies in addition to branches in the
>> > list-of-branches view.
>> >
>> > Signed-off-by: Krzesimir Nowak <krzesimir@endocode.com>
>> 
>> Why not use %feature hash instead of adding new configuration variable?
>> I think that this option is similar enough to 'remote_heads' feature
>> (which BTW should be 'remote-heads'), and could conceveilably enabled
>> on a per-repository basis, i.e. with repository configuration override,
>> isn't it?
>
> I'd like to see some consensus on it before I start changing the patch
> again.

I missed the remote-heads which is an existing feature when I commented; if this can be exposed to the users as an extension to it like Jakub suggests, it may be a better direction.

Show 6 quoted lines

>> Usually %feature hash is preferred over adding new configuration variable
>> but this is not some hard rule. Note however that patches adding new config
>> are met with more scrutiny, as it is harder to fix mistakes because of
>> requirement of backwards compatibility of configuration files.
>
> I don't know what kind of backwards compatibility you mention.

A patch adds new feature controlled by a configuration in one way (e.g. as a totally new ad-hoc switch that is seemingly orthogonal to all the existing features), people start using the feature using that configuration method, and then later we find out that it is better controlled by a different way (e.g. as a natural extension to an existing feature, controlled by how the existing feature is controlled, perhaps after extending it) because it allows more flexibility in the future.

At that point, we can extend the way the existing feature is controlled to trigger the new feature in a more uniform way, but we cannot remove the new ad-hoc switch the patch originally added to control this new feature because there already are people who are using it, and we end up having to support the unified and extensible way to configure, which we prefer to keep in the longer term, and also the ad-hoc switch the patch added, which we wish we never had in the first place. The latter needs to be deprecated and removed over time.