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

[Eric Sunshine <sunshine@sunshineco.com>]

On Wed, Nov 27, 2013 at 3:34 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Wed, Nov 27, 2013 at 10:30 AM, Krzesimir Nowak
> <krzesimir@endocode.com> wrote:
>> Overriding an @additional_branch_refs configuration variable with
>> value ('wip') will make gitweb to show branches that appear in
>> refs/heads and refs/wip (refs/heads is hardcoded). Might be useful for
>> gerrit setups where user branches are not stored under refs/heads/.
>>
>> Signed-off-by: Krzesimir Nowak <krzesimir@endocode.com>
>> ---
>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
>> index 68c77f6..499281b 100755
>> --- a/gitweb/gitweb.perl
>> +++ b/gitweb/gitweb.perl
>> @@ -680,6 +688,19 @@ sub read_config_file {
>>         return;
>>  }
>>
>> +# performs sanity checks on parts of configuration.
>> +sub config_sanity_check {
>> +       # check additional refs validity
>> +       my %unique_branch_refs = ();
>> +       for my $ref (@additional_branch_refs) {
>> +               die_error(500, 'Invalid ref in @additional_branch_refs') unless (validate_ref($ref));
>
> Mentioning $ref in the error message would help the user resolve the
> problem more quickly.
>
>> +               die_error(500, '"heads" specified in @additional_branch_refs') if ($ref eq 'heads');
>
> Rephrasing this as
>
>     "heads" disallowed in @additional_branch_refs
>
> would better explain the problem to a user who has only made a cursory
> read of the documentation.

The program could easily filter out the redundant 'heads', so does
this really deserve a diagnostic?