Update perlrepository.pod with new guidelines on commit messages and patches.

Karl Williamson pointed out to me that our existing guidelines on
commit messages in 'perlrepository.pod' were somewhat less than ideal.

This commit improves our documentation for commit messages, though it
may want cleanup or further improvement.

diff --git a/pod/perlrepository.pod b/pod/perlrepository.pod
index 6bd3048..764348c 100644 (file)
--- a/pod/perlrepository.pod
+++ b/pod/perlrepository.pod
@@ -227,13 +227,27 @@ not been pushed to the C<origin> remote yet. B<NOTE>: that this output
 is also what you see as a template if you do not provide a message to
 C<git commit>.
 
-Assuming we commit all the mentioned changes above:
+Assuming that you'd like to commit all the changes you've just made as a
+a single atomic unit, run this command:
+
+   % git commit -a
+
+(That C<-a> tells git to add every file you've changed to this commit.
+If you want to commit some, but not all of your changes, have a look
+at the documentation for C<git add>.)
+
+Git will start up your favorite text message, so that you can craft a
+commit message for your change. See L</Commit message> below for more
+information about what makes a good commit message.
+
+Once you've finished writing your commit message and exited your editor,
+git will write your change to disk and tell you something like this:
 
-  % git commit -a -m'explain git status and stuff about remotes'
   Created commit daf8e63: explain git status and stuff about remotes
    1 files changed, 83 insertions(+), 3 deletions(-)
 
-We can re-run git status and see something like this:
+
+If you re-run C<git status>, you should see something like this:
 
   % git status
   # On branch blead
@@ -391,17 +405,51 @@ What should we recommend about binary files now? Do we need anything?
 
 =head2 Getting your patch accepted
 
-The first thing you should include with your patch is a description of
-the problem that the patch corrects.  If it is a code patch (rather
-than a documentation patch) you should also include a small test case
-that illustrates the bug (a patch to an existing test file is
-preferred).
-
-If you are submitting a code patch there are several other things that
+If you are submitting a code patch there are several things that
 you need to do.
 
 =over 4
 
+=item Commit message
+
+As you craft each patch you intend to submit to the Perl core, it's
+important to write a good commit message.
+
+Your commit message should start with a description of the problem that
+the patch corrects or new functionality that the patch adds.
+
+
+As a general rule of thumb, your commit message should let a programmer
+with a reasonable familiarity with the Perl core quickly understand what
+you were trying to do, how you were trying to do it and why the change
+matters to Perl.
+
+=over 4
+
+=item What
+
+Your commit message should describe what part of the Perl core you're changing and what you expect your patch to do.
+
+=item Why
+
+Perhaps most importantly, your commit message should describe why the
+change you are making is important. When someone looks at your change
+in six months or six years, your intent should be clear.  If you're
+deprecating a feature with the intent of later simplifying another bit
+of code, say so. If you're fixing a performance problem or adding a new
+feature to support some other bit of the core, mention that.
+
+=item How
+
+While it's not necessary for documentation changes, new tests or
+trivial patches, it's often worth explaining how your change works.
+Even if it's clear to you today, it may not be clear to a porter next
+month or next year.
+
+=back
+
+
+
 =item Comments, Comments, Comments
 
 Be sure to adequately comment your code.  While commenting every line
@@ -439,8 +487,11 @@ sources:
 
 =item Testsuite
 
-When submitting a patch you should make every effort to also include an
-addition to perl's regression tests to properly exercise your patch.
+If your patch changes code (rather than just changing documentation) you
+should also include one or more test cases which illustrate the bug you're
+fixing or validate the new functionality you're adding.  In general,
+you should update an existing test file rather than create a new one.
+
 Your testsuite additions should generally follow these guidelines
 (courtesy of Gurusamy Sarathy <gsar@activestate.com>):
 
@@ -559,7 +610,7 @@ C<git> provides a built-in way to determine, with a binary search in
 the history, which commit should be blamed for introducing a given bug.
 
 Suppose that we have a script F<~/testcase.pl> that exits with C<0>
-when some behaviour is correct, and with C<1> when it's faulty. We need
+when some behaviour is correct, and with C<1> when it's faulty. You need
 an helper script that automates building C<perl> and running the
 testcase:
 
@@ -586,7 +637,7 @@ This script may return C<125> to indicate that the corresponding commit
 should be skipped. Otherwise, it returns the status of
 F<~/testcase.pl>.
 
-We first enter in bisect mode with:
+You first enter in bisect mode with:
 
   % git bisect start
 
@@ -598,7 +649,7 @@ C<git> will learn about this when you enter:
   Bisecting: 853 revisions left to test after this
 
 This results in checking out the median commit between C<HEAD> and
-C<perl-5.10.0>. We can then run the bisecting process with:
+C<perl-5.10.0>. You can then run the bisecting process with:
 
   % git bisect run ~/run
 
@@ -643,7 +694,7 @@ you should clone:
 
   % git clone git@github.com:USERNAME/perl.git perl-github
 
-We shall make the same patch as above, creating a new branch:
+The same patch as above, using github might look like this:
 
   % cd perl-github
   % git remote add upstream git://github.com/github/perl.git
@@ -784,6 +835,8 @@ C<.git/info/grafts> file:
 It is particularly important to have this graft line if any bisecting
 is done in the area of the "merge" in question.
 
+
+
 =head1 SEE ALSO
 
 The git documentation, accessible via C<git help command>.