Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 23 Jan 2010 22:50:25 -0800
From:      Doug Barton <dougb@FreeBSD.org>
To:        =?ISO-8859-1?Q?Francisco_de_Borja_L=F3pez_R=EDo?= <borja@pexego.es>
Cc:        ports@freebsd.org, wen heping <wenheping@gmail.com>
Subject:   Re: New port: finance/openerp-web
Message-ID:  <4B5BEDB1.8080308@FreeBSD.org>
In-Reply-To: <20100121095114.12bab6af.borja@pexego.es>
References:  <20100120165147.666592ef.borja@pexego.es>	<20100120212640.GE6618@lonesome.com>	<7be7a2801001201600j5c772267na6dea944334618be@mail.gmail.com>	<20100121093649.7867d810.borja@pexego.es> <20100121095114.12bab6af.borja@pexego.es>

next in thread | previous in thread | raw e-mail | index | archive | help
On 01/21/10 00:51, Francisco de Borja López Río wrote:
> 
> Sorry, I forgot to add the attachments in my previous email.
> 
> These are both the rc.d script and the default configuration file I've
> been using with the current openerp-server port for some time.

A few notes on the rc.d script.

1. The name of the file, PROVIDE, and $name should all match. Perhaps
openerpd, or even erpd?
2. There are quite a few examples of "/usr/local" in there (which is
fine for a running script), please remember to s#/usr/local#%%PREFIX%%#g
for the port.
3. The pidfile= should come after load_rc_config, and should look like
this:
pidfile="${<name>_pidfile:-/var/run/openerp-server/openerp-server.pid}"
(substitute <name> of course). Or, consider not allowing the user to
change the location of the pidfile unless there is a good reason to do so.
4. I'm not quite sure what eval "${rcvar}=\${${rcvar}:-'NO'}" is
supposed to accomplish, but if it's "set ${name}_enable to NO by
default" it's too clever by half. :)  Take a look at
http://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/rc-scripts.html
for other ways to simplify the default variable assignments (as well as
some other pointers).
5. Just doing 'touch foo' is always cheaper than testing its existence
first (although it's a micro-optimization) and there is no reason to
test existence anyway. Same goes with the 'mkdir -p'.
6. Instead of `dirname blah` you should use ${blah%/*} for two reasons,
first dirname(1) is in /usr/bin, and may not be available at boot, and
two there is no reason to use it in a shell script when the variable
substitution is cheaper and easier.
7. You named your substitute stop_cmd "${name}_poststop" which is
confusing, and I'm not sure why it's necessary. If you're afraid that
rc.d isn't going to actually stop the process for some reason we should
address that. If you feel that you absolutely must have a safety belt
for this make it a real stop_postcmd.

I realize that these are a lot of notes, but don't worry ... it's easy
to see that a lot of thought and creativity went into this, which is why
I'm taking the time to try and help improve it.


Doug

-- 

	Improve the effectiveness of your Internet presence with
	a domain name makeover!    http://SupersetSolutions.com/

	Computers are useless. They can only give you answers.
			-- Pablo Picasso




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4B5BEDB1.8080308>