2010/9/26 Jakub Narebski <jnareb@xxxxxxxxx>:
> On Fri, 24 Sep 2010, Giuseppe Bilotta wrote:
>>
>> +# returns a submenu for the nagivation of the refs views (tags, heads,
>> +# remotes) with the current view disabled and the remotes view only
>> +# available if the feature is enabled
>> +
>
> Minor nitpick: this empty line here is not necessary. But I think
> that Junio can remove it when applying.
Since there have been a couple of stylistic suggestions for the
comments in the first two patches too, I can probably resend the whole
series including these changes, unless Junio wants to do the
hand-tuning.
>> +sub format_ref_views {
>> + my ($current) = @_;
>> + my @ref_views = qw{tags heads};
>
> Hmmm... should we pass it as argument, or use $action in place of
> $current? Each solution has its advantages and disadvantages. Current
> solution has the advantage of avoiding using global variables, solution
> using $action has the (supposed) advantage of automatically detecting
> current action.
Not using $action has the advantage of making it possible to enable
the $action command if it's wanted, which is something that I use in a
subsequent patch (when enabling single-remote view). But this is of
course debatable.
--
Giuseppe "Oblomov" Bilotta
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html