For the rc.d scripts section:

Move load_rc_config and the default variables up to the newly preferred
location, right after rcvar.

Minor grammar fixes.

Add a pre-commit checklist that has most of the nitpicks that I do by hand.
This commit is contained in:
Doug Barton 2011-12-02 06:53:44 +00:00
parent dd0a9cd26c
commit 41b932175e
Notes: svn2git 2020-12-08 03:00:23 +00:00
svn path=/head/; revision=37987

View file

@ -8743,21 +8743,20 @@ CFLAGS+= -DLUA_VERSION_STRING="${VER_STR}"
# Set it to YES to enable doormand.
# doormand_config (path): Set to %%PREFIX%%/etc/doormand/doormand.cf
# by default.
#
. /etc/rc.subr
name="doormand"
rcvar=${name}_enable
command=%%PREFIX%%/sbin/${name}
pidfile=/var/run/${name}.pid
load_rc_config $name
: ${doormand_enable="NO"}
: ${doormand_enable:="NO"}
: ${doormand_config="%%PREFIX%%/etc/doormand/doormand.cf"}
command=%%PREFIX%%/sbin/${name}
pidfile=/var/run/${name}.pid
command_args="-p $pidfile -f $doormand_config"
run_rc_command "$1"</programlisting>
@ -8772,7 +8771,8 @@ run_rc_command "$1"</programlisting>
down cleanly when the system shuts down. If the script is not
starting a persistent service this is not necessary.</para>
<para>The &quot;=&quot; style of default variable assignment
<para>For optional configuration elements
the &quot;=&quot; style of default variable assignment
is preferable to the &quot;:=&quot; style here, since the
former sets a default value only if the variable is unset,
and the latter sets one if the variable is unset
@ -8799,13 +8799,118 @@ run_rc_command "$1"</programlisting>
whether to stop the service on deinstall or not. Also note this
affects upgrades, too.</para>
<para>Line like this goes to the <filename>pkg-plist</filename>:</para>
<para>A line like this goes in the <filename>pkg-plist</filename>:</para>
<programlisting>@stopdaemon doormand</programlisting>
<para>The argument must match the content of
<makevar>USE_RC_SUBR</makevar> variable.</para>
</sect2>
<sect3>
<title>Pre-Commit Checklist</title>
<para>Before contributing a port with an <filename>rc.d</filename>
script, and more importantly, before commiting one; please consult
the following checklist to be sure that it is ready.</para>
<procedure>
<step>
<para>If this is a new file, does it have <literal>.sh</literal>
in the file name? If so that should be changed to just
<filename>file.in</filename> since new rc.d files may not end
with that extension.</para>
</step>
<step>
<para>Does the file have a $dollar;FreeBSD$dollar; tag?</para>
</step>
<step>
<para>Do the name of the file (minus <literal>.in</literal>),
the PROVIDE line, and &dollar;name all match? The file name
matching PROVIDE makes debugging easier, especially for
&man.rcorder.8; issues. Matching the file name and
&dollar;name makes figuring out what variables in
rc.conf[.local] are relevant easier as well. The latter is
also what you might call &quot;policy&quot; for all new
scripts, including in the base.</para>
</step>
<step>
<para>Is the REQUIRE line set to LOGIN? This is mandatory for
scripts that run as a non-root user. If it runs as root, is
there a good reason for it to run prior to LOGIN? If not, it
should run there so that we can loosely group local scripts
to a point in &man.rcorder.8; after most everything in the
base is already running.</para>
</step>
<step>
<para>Does the script start a persistent service? If so, it
should have KEYWORD: shutdown.</para>
</step>
<step>
<para>Make sure there is no KEYWORD: FreeBSD present. This has
not been necessary or desirable for years. It is also an
indication that the new script was copy/pasted from an old
script, so extra caution should be given to the review.</para>
</step>
<step>
<para>Does the script use an interpreted language (perl, python,
ruby, etc.)? If so, is command_interpreter set appropriately?
If not, it is likely that
<programlisting>&man.service.8; script stop</programlisting>
will not work properly.</para>
</step>
<step>
<para>Are all examples of /usr/local subbed out for %%PREFIX%%?</para>
</step>
<step>
<para>Do the default variable assignments come after
load_rc_config?</para>
</step>
<step>
<para>Are there default assignments to empty strings?
They should be removed, but double-check that the option is
documented in the comments at the top of the file.</para>
</step>
<step>
<para>Are things that are set in variables actually used in
the script?</para>
</step>
<step>
<para>Are options listed in the default _flags things that are
actually mandatory? If so, they should be in command_args.
The -d option is a red flag (pardon the pun) here, since it's
usually the option to &quot;daemonize&quot; the process, and
therefore is actually mandatory.</para>
</step>
<step>
<para>The _flags variable should never be included in command_args
(and vice versa, although that error is less common).</para>
</step>
<step>
<para>Does the script execute any code unconditionally? This
is frowned on. Usually these things can/should be dealt with
through a start_precmd.</para>
</step>
<step>
<para>All boolean tests should utilize the checkyesno function.
No hand-rolled tests for [Yy][Ee][Ss], etc.</para>
</step>
</procedure>
</sect3>
</sect1>
<sect1 id="users-and-groups">