Differential D16730 Diff 53855 head/en_US.ISO8859-1/articles/committers-guide/article.xml

Changeset View

Standalone View

head/en_US.ISO8859-1/articles/committers-guide/article.xml

Show First 20 Lines • Show All 2,374 Lines • ▼ Show 20 Lines	<tbody>
<entry><filename>ports/svnadmin/conf/mentors</filename></entry>		<entry><filename>ports/svnadmin/conf/mentors</filename></entry>
</row>		</row>
</tbody>		</tbody>
</tgroup>		</tgroup>
</informaltable>		</informaltable>
</sect2>		</sect2>
</sect1>		</sect1>

		<sect1 xml:id="pre-commit-review">
		<title>Pre-Commit Review</title>

		<para>Code review is one way to increase the quality of software.
		The following guidelines apply to commits to the
		<literal>head</literal> (-CURRENT) branch of the
		<literal>src</literal> repository. Other branches and the
		<literal>ports</literal> and <literal>docs</literal> trees have
		their own review policies, but these guidelines generally apply
		to commits requiring review:</para>
		<itemizedlist>
		<listitem>
		<para>All non-trivial changes should be reviewed before they
		are committed to the repository.</para>
		</listitem>

		<listitem>
		<para>Reviews may be conducted by email, in
		<application>Bugzilla</application>, in
		<application>Phabricator</application>, or by another
		mechanism. Where possible, reviews should be public.</para>
		</listitem>

		<listitem>
		<para>The developer responsible for a code change is also
		responsible for making all necessary review-related
		changes.</para>
		</listitem>

		<listitem>
		<para>Code review can be an iterative process, which continues
		until the patch is ready to be committed. Specifically,
		once a patch is sent out for review, it should receive an
		explicit <quote>looks good</quote> before it is committed.
		So long as it is explicit, this can take whatever form makes
		sense for the review method.</para>
		</listitem>

		<listitem>
		<para>Timeouts are not a substitute for review.</para>
		</listitem>
		</itemizedlist>

		<para>Sometimes code reviews will take longer than you would hope
		for, especially for larger features. Accepted ways to speed up
		review times for your patches are:</para>

		<itemizedlist>
		<listitem>
		<para>Review other people's patches. If you help out,
		everybody will be more willing to do the same for you;
		goodwill is our currency.</para>
		</listitem>

		<listitem>
		<para>Ping the patch. If it is urgent, provide reasons why
		it is important to you to get this patch landed and ping
		it every couple of days. If it is not urgent, the common
		courtesy ping rate is one week. Remember that you are
		asking for valuable time from other professional
		developers.</para>
		</listitem>

		<listitem>
		<para>Ask for help on mailing lists, IRC, etc. Others
		may be able to either help you directly, or suggest a
		reviewer.</para>
		</listitem>

		<listitem>
		<para>Split your patch into multiple smaller patches that
		build on each other. The smaller your patch, the higher
		the probability that somebody will take a quick look at
		it.</para>

		<para>When making large changes, it is helpful to keep this
		in mind from the beginning of the effort as breaking large
		changes into smaller ones is often difficult after the
		fact.</para>
		</listitem>
		</itemizedlist>

		<para>Developers should participate in code reviews as both
		reviewers and reviewees. If someone is kind enough to review
		your code, you should return the favor for someone else.
		Note that while anyone is welcome to review and give feedback
		on a patch, only an appropriate subject-matter expert can
		approve a change. This will usually be a committer who works
		with the code in question on a regular basis.</para>

		<para>In some cases, no subject-matter expert may be available.
		In those cases, a review by an experienced developer is
		sufficient when coupled with appropriate testing.</para>
		</sect1>

<sect1 xml:id="commit-log-message">		<sect1 xml:id="commit-log-message">
<title>Commit Log Messages</title>		<title>Commit Log Messages</title>

<para>This section contains some suggestions and traditions for		<para>This section contains some suggestions and traditions for
how commit logs are formatted.</para>		how commit logs are formatted.</para>

<para>As well as including an informative message with each		<para>As well as including an informative message with each
commit, some additional information may be needed.</para>		commit, some additional information may be needed.</para>
▲ Show 20 Lines • Show All 2,946 Lines • Show Last 20 Lines