Date: Tue, 31 Aug 2010 10:27:06 -0700 From: Doug Barton <dougb@FreeBSD.org> To: Martin Matuska <mm@FreeBSD.org> Cc: cvs-ports@FreeBSD.org, cvs-all@FreeBSD.org, ports-committers@FreeBSD.org Subject: Re: cvs commit: ports/www/lighttpd Makefile ports/www/lighttpd/files lighttpd.sh.in Message-ID: <4C7D3B6A.5070701@FreeBSD.org> In-Reply-To: <201008311011.o7VABx0K050327@repoman.freebsd.org> References: <201008311011.o7VABx0K050327@repoman.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
There are a couple of problems with this script, none of them major. First, there are several empty variable assignments, which are neither necessary nor desirable. There are great documentation in the comments, which makes them even less necessary. Second, you have 2 big blocks of code that run unconditionally which we really try to avoid. The deprecated_conf stuff should probably be in a start_precmd method. The lighttpd_instances stuff is harder to call, but it should probably be a start_cmd method. The check_deprecated method should use warn() which has the advantage of also logging the message. Last but not least, run_instance() should use local variables. hth, Doug On 8/31/2010 3:11 AM, Martin Matuska wrote: > mm 2010-08-31 10:11:59 UTC > > FreeBSD ports repository > > Modified files: > www/lighttpd Makefile > www/lighttpd/files lighttpd.sh.in > Log: > - Add multiple instances ability to rc script > > Revision Changes Path > 1.90 +1 -1 ports/www/lighttpd/Makefile > 1.9 +57 -7 ports/www/lighttpd/files/lighttpd.sh.in > > http://www.FreeBSD.org/cgi/cvsweb.cgi/ports/www/lighttpd/Makefile.diff?&r1=1.89&r2=1.90&f=h > http://www.FreeBSD.org/cgi/cvsweb.cgi/ports/www/lighttpd/files/lighttpd.sh.in.diff?&r1=1.8&r2=1.9&f=h >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4C7D3B6A.5070701>