Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 17 Mar 2010 03:12:05 +0100
From:      Martin Pala <martinp@tildeslash.com>
To:        Doug Barton <dougb@FreeBSD.org>
Cc:        Wen Heping <wen@FreeBSD.org>, cvs-ports@FreeBSD.org, cvs-all@FreeBSD.org, ports-committers@FreeBSD.org
Subject:   Re: cvs commit: ports/sysutils/monit/files monit.sh.in
Message-ID:  <0E8E3FB7-40B8-4666-A3B6-462621F43B98@tildeslash.com>
In-Reply-To: <4B9EEFA3.80106@FreeBSD.org>
References:  <201003160209.o2G29ieE041601@repoman.freebsd.org> <4B9EEFA3.80106@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help


On Mar 16, 2010, at 3:40 AM, Doug Barton wrote:

> On 03/15/10 19:09, Wen Heping wrote:
>> wen         2010-03-16 02:09:44 UTC
>> 
>>  FreeBSD ports repository
>> 
>>  Modified files:
>>    sysutils/monit/files monit.sh.in 
>>  Log:
>>  - Improve rc script
>> 
>>  PR:             ports/144772  http://www.FreeBSD.org/cgi/query-pr.cgi?pr=144772
>>  Submitted by:   Martin Pala <martinp@tildeslash.com> (maintainer)
>> 
>>  Revision  Changes    Path
>>  1.2       +7 -5      ports/sysutils/monit/files/monit.sh.in
>> 
>> http://www.FreeBSD.org/cgi/cvsweb.cgi/ports/sysutils/monit/files/monit.sh.in.diff?&r1=1.1&r2=1.2&f=h
> 
> Documenting the _flags option in comments is an improvement, yes.
> Everything else is a pessimization; particularly since the default
> assignment for monit_enable is now broken.
> 
> I've attached a patch that fixes the following issues:
> 1. General re-sorting to match conventions.
> 2. Change the default for _enable to the conventional method, and fix it
> as a side effect. Also move it down past load_rc_config. Otherwise
> testing for a value first is meaningless.
> 3. Eliminate the need for $default_config
> 
> Martin, please test this and respond ASAP. Since the script as committed
> is now broken, it needs to be fixed.
> 

Hi,

no, the script is not broken - it works the same as before, i have tested it and found no issues - if you can reproduce any problem, please provide details. The monit_enable works OK with both values. The testing of monit_enable value before load_rc_config works OK - it should always fail and the monit_enable is set to "NO", which can be overridden by load_rc_config (is executed post this setting).

I think your patch is not necessary - it does cosmetic modifications only. For example apache rc script has very similar structure as monit rc script.

Regards,
Martin





Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?0E8E3FB7-40B8-4666-A3B6-462621F43B98>