Re: [PATCHv2 0/2] fixes to gitweb feature check code

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Dnia piątek 28. listopada 2008 21:39, Giuseppe Bilotta napisał:
> This is v2 of the gitweb feature check fix patch, which has now been
> split into into a code patch and cleanup patch.
> 
> The first patch introduces git_get_feature() to clearly distinguish the
> feature retrieval from the boolean feature check (which is kept at
> git_check_feature()). The new function is used where appropriate.
> 
> The second patch cleans up use of git_check_feature(): since the
> function now returns a boolean instead of an array, the often-used
> construct
>   my ($somevar) = git_check_feature('somefeat');
> although still valid, becomes a rather clumsy stylistic choice, as it
> introduces an unnecessary ambiguity. Make it clear that we're now
> dealing with scalars by using scalar assignment.

It is gitweb_get_feature() (for retrieval) and gitweb_check_feature()
(for checking), not git_get_feature() and git_check_feature().

I really like this series; making coding easier by removing need to
use special construct, and making it easier to avoid mistakes is always
a good thing.

> Giuseppe Bilotta (2):
>   gitweb: fixes to gitweb feature check code
>   gitweb: clean up git_check_feature() calls
> 
>  gitweb/gitweb.perl |   52 +++++++++++++++++++++++++++++++++++-----------------
>  1 files changed, 35 insertions(+), 17 deletions(-)

I also like the split, as it separates improvement (much easier to use
and less error prone gitweb_check_feature()) from style cleanup.

-- 
Jakub Narebski
Poland
--
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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux