From: Giuseppe Bilotta Date: Mon, 27 Sep 2010 06:48:30 GMT Subject: Re: [PATCHv5 04/12] gitweb: nagivation menu for tags, heads and remotes Message-ID: In-Reply-To: <201009261952.07803.jnareb@gmail.com> 2010/9/26 Jakub Narebski : > 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