All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jakub Narebski <jnareb@gmail.com>
To: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Cc: git@vger.kernel.org, Petr Baudis <pasky@suse.cz>,
	Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH] gitweb: fixes to gitweb feature check code
Date: Mon, 17 Nov 2008 02:02:26 +0100	[thread overview]
Message-ID: <200811170202.27893.jnareb@gmail.com> (raw)
In-Reply-To: <1226759165-6894-1-git-send-email-giuseppe.bilotta@gmail.com>

On Sat, 15 Nov 2008, Giuseppe Bilotta wrote:

> The gitweb_check_feature routine was being used for two different
> purposes: retrieving the actual feature value (such as the list of
> snapshot formats or the list of additional actions), and to check if a
> feature was enabled.
> 
> For the latter use, since all features return an array, it led to either
> clumsy code or subtle bugs, with disabled features appearing enabled
> because (0) evaluates to 1.
> 
> We fix these bugs, and simplify the code, by separating feature (list)
> value retrieval (gitweb_get_feature) from boolean feature check (i.e.
> retrieving the first/only item in the feature value list). Usage of
> gitweb_check_feature across gitweb is replaced by the appropriate call
> wherever needed.
> ---

First, you forgot the signoff, but you have addressed that already.


Second, I thought at first that it would be good for the patch to also
simplify %feature hash, using "'default' => 1" instead of current bit
convoluted "'default' => [1]", at the cost of bit more code for
defensive programming.  But now I think that if it is to be done,
it should be put as separate patch.

-- 
Jakub Narebski
Poland

  parent reply	other threads:[~2008-11-17  1:03 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-11-15 14:26 [PATCH] gitweb: fixes to gitweb feature check code Giuseppe Bilotta
2008-11-16 15:30 ` Giuseppe Bilotta
2008-11-16 21:11 ` Junio C Hamano
2008-11-16 21:57   ` Giuseppe Bilotta
2008-11-17  1:02 ` Jakub Narebski [this message]
2008-11-17  6:10   ` Giuseppe Bilotta
2008-11-17  9:28     ` Jakub Narebski
2008-11-17 10:09       ` Junio C Hamano
2008-11-17 10:48         ` Jakub Narebski
     [not found] <Message-ID: <cb7bb73a0811291731g7f8770f7p89e924c00d2ab004@mail.gmail.com>
2008-11-30  1:31 ` [PATCH 3/3] gitweb: make gitweb_check_feature a boolean wrapper Giuseppe Bilotta
2008-11-30  1:34 ` [PATCH] gitweb: fixes to gitweb feature check code Giuseppe Bilotta
2008-12-02  1:53   ` Jakub Narebski
2008-12-02 21:55     ` Junio C Hamano
2008-12-03  1:21       ` Jakub Narebski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200811170202.27893.jnareb@gmail.com \
    --to=jnareb@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=giuseppe.bilotta@gmail.com \
    --cc=pasky@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.