Re: [PATCH 3/3] git-instaweb: Don't assume Apache executable is named apache2

From: Eric Wong <normalperson@yhbt.net>
Date: Jun 30, 2010, 07:31 UTC
Message-ID: <20100630073153.GA25218@dcvr.yhbt.net>
In-Reply-To: <1277865900-25044-3-git-send-email-dpmcgee@gmail.com>

Dan McGee <dpmcgee@gmail.com> wrote:

Show 5 quoted lines

> On Arch Linux, we keep the original 'httpd' name for the exectuable and
> don't rename it or the path to the modules. Remove some of these assumptions
> and add the httpd name in where it may be required.
> 
> Signed-off-by: Dan McGee <dpmcgee@gmail.com>

Thanks Dan,

Everything in this series looks reasonable to me (haven't tested):

Acked-by: Eric Wong <normalperson@yhbt.net>

Show 8 quoted lines

> --- a/git-instaweb.sh
> +++ b/git-instaweb.sh
>  apache2_conf () {
> -	test -z "$module_path" && module_path=/usr/lib/apache2/modules
> +	if test -z "$module_path"; then
> +		test -d "/usr/lib/httpd/modules" && module_path="/usr/lib/httpd/modules"
> +		test -d "/usr/lib/apache2/modules" && module_path="/usr/lib/apache2/modules"
> +	fi

Minor style nitpicks (feel free to ignore unless somebody else feels more strongly):

The preferred style in git is to have the "then" statement on a separate line, omitting the semi-colon, but there remain plenty of examples contrary in our code:

Likewise with "for i in x y z; do"

We also prefer to keep lines shorter than 80 characters (hard tabs being 8 chars), but we also have plenty of examples contrary to that in instaweb, too :x

-- 
Eric Wong