os/doc
3
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Add markup and minor edits for clarity to the new Pre-Commit Checklist

Browse source 
section.

Reviewed by:	dougb
Approved by:	gjb (mentor)
This commit is contained in:
Warren Block 2011-12-11 05:09:04 +00:00
parent 7e74462824
commit acdf4869f9
Notes: svn2git 2020-12-08 03:00:23 +00:00 
svn path=/head/; revision=38034
1 changed files with 47 additions and 28 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

75
en_US.ISO8859-1/books/porters-handbook/book.sgml
View file

@ -8827,7 +8827,7 @@ run_rc_command "$1"</programlisting>
<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
script, and more importantly, before committing one, please consult
the following checklist to be sure that it is ready.</para>
<procedure>
@ -8845,19 +8845,24 @@ run_rc_command "$1"</programlisting>
<step>
<para>Do the name of the file (minus <filename>.in</filename>),
the PROVIDE line, and &dollar;name all match? The file name
matching PROVIDE makes debugging easier, especially for
the <literal>PROVIDE</literal> line, and
<literal>&dollar;</literal><replaceable>name</replaceable> all
match? The file name matching <literal>PROVIDE</literal>
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>
<literal>&dollar;</literal><replaceable>name</replaceable>
makes it easier to figure out which variables are relevant in
<filename>rc.conf[.local]</filename>. The latter
is also what you might call &ldquo;policy&rdquo; for all new
scripts, including those in the base system.</para>
</step>
<step>
<para>Is the REQUIRE line set to LOGIN? This is mandatory for
<para>Is the <literal>REQUIRE</literal> line set to
<literal>LOGIN</literal>? 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
there a good reason for it to run prior to
<literal>LOGIN</literal>? 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>
@ -8865,31 +8870,36 @@ run_rc_command "$1"</programlisting>
<step>
<para>Does the script start a persistent service? If so, it
should have KEYWORD: shutdown.</para>
should have <literal>KEYWORD: shutdown</literal>.</para>
</step>
<step>
<para>Make sure there is no KEYWORD: FreeBSD present. This has
<para>Make sure there is no <literal>KEYWORD: FreeBSD</literal>
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>
<para>If the script uses an interpreted language like
<command>perl</command>, <command>python</command>, or
<command>ruby</command>, make certain that
<varname>command_interpreter</varname> is set appropriately.
Otherwise,
<programlisting><command>service</command> <replaceable>name</replaceable> stop</programlisting>
will probably not work properly.</para>
</step>
<step>
<para>Are all examples of /usr/local subbed out for %%PREFIX%%?</para>
<para>Have all occurrences of
<filename>/usr/local</filename> been replaced with
<literal>%%PREFIX%%</literal>?</para>
</step>
<step>
<para>Do the default variable assignments come after
load_rc_config?</para>
<function>load_rc_config</function>?</para>
</step>
<step>
@ -8904,27 +8914,34 @@ run_rc_command "$1"</programlisting>
</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 is
usually the option to &quot;daemonize&quot; the process, and
<para>Are options listed in the default
<replaceable>name</replaceable><varname>_flags</varname>
things that are actually mandatory? If so, they should be in
<varname>command_args</varname>.
The <option>-d</option> option is a red flag (pardon the pun)
here, since it is
usually the option to &ldquo;daemonize&rdquo; the process, and
therefore is actually mandatory.</para>
</step>
<step>
<para>The _flags variable should never be included in command_args
<para>The <replaceable>name</replaceable><varname>_flags</varname>
variable should never be included in
<varname>command_args</varname>
(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>
through a <function>start_precmd</function>.</para>
</step>
<step>
<para>All boolean tests should utilize the checkyesno function.
No hand-rolled tests for [Yy][Ee][Ss], etc.</para>
<para>All boolean tests should utilize the
<function>checkyesno</function> function.
No hand-rolled tests for <literal>[Yy][Ee][Ss]</literal>,
etc.</para>
</step>
<step>
@ -8935,9 +8952,11 @@ run_rc_command "$1"</programlisting>
<step>
<para>Does the script create files or directories that need
specific permissions, for example, a pid file that needs
specific permissions, for example, a <filename>pid</filename>
file that needs
to be owned by the user that runs the process? Rather than
the traditional touch/chown/chmod routine consider using
the traditional &man.touch.1;/&man.chown.8;/&man.chmod.1;
routine, consider using
&man.install.1; with the proper command line arguments to
do the whole procedure with one step.</para>
</step>