Re: [PATCHv5 04/12] gitweb: nagivation menu for tags, heads and remotes
- From
Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
- Date
- Sep 27, 2010, 06:48 UTC
- Message-ID
- <AANLkTi=nG9Day4ZCF88JwyeNx2qEM=+hKaKoyWRSTUFW@mail.gmail.com>
- In-Reply-To
- <201009261952.07803.jnareb@gmail.com>
2010/9/26 Jakub Narebski <jnareb@gmail.com>:
Show 9 quoted lines
> 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.
Show 9 quoted lines
>> +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