git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Adam Monsen <haircut@gmail.com>
To: git@vger.kernel.org
Cc: Jeff King <peff@peff.net>, Junio Hamano <gitster@pobox.com>,
	Jakub Narebski <jnareb@gmail.com>,
	Adam Monsen <haircut@gmail.com>
Subject: [PATCH] SubmittingPatches: clean up commit message tips
Date: Wed,  9 Mar 2011 13:27:49 -0800	[thread overview]
Message-ID: <1299706069-5463-1-git-send-email-haircut@gmail.com> (raw)
In-Reply-To: <4D77F03B.4050605@gmail.com>

Removed uncommon acronyms in the "Checklist" section. I had to look up
"iow" online, but this documentation should stand alone (without online
access) as well as commit messages.

Leave wiggle room for including URLs in commit messages. Having both
summaries of mailing list discussions as well as URLs in commit messages
gives the reader the choice of how deep into history they wish to dig.

Modify the section about trivial changes slightly... it makes more sense
that it is discouraging diffs pasted in emails as opposed to patches
generated with "git am".

Remove recommendations on commit messages from the "Make separate
commits for logically separate changes" section, they are covered
well and explicitly in the "Checklist" section on top. This section need
not repeat same, and the deleted text doesn't really apply to the
section it was under. Also remove irrelevant text about good commit
messages. The only relevant part of that paragraph was the first
sentence about breaking apart big commits into separate patches.

Consistently use the phrase "commit message" to mean the chunk of text
that describes a commit.

Also in "Checklist", make the third bullet under "...a meaningful
commit message, which:" flow better grammatically by adding "mentions".

Relevant mailing list discussion:

http://thread.gmane.org/gmane.comp.version-control.git/168481/focus=168717

Signed-off-by: Adam Monsen <haircut@gmail.com>
---

I realize that this commit probably violates the very virtues it recommends by
having a long commit message and fixing several things in one shot. Let me know
if you'd like me to break it apart. Also, I may of course have made further
errors. It does adhere to the subject of cleaning up commit message tips.

Hope this helps,
-Adam

 Documentation/SubmittingPatches |   38 ++++++++++++++++----------------------
 1 files changed, 16 insertions(+), 22 deletions(-)

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index c3b0816..3f8bff5 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -10,18 +10,19 @@ Checklist (and a short version for the impatient):
 	  description (50 characters is the soft limit, see DISCUSSION
 	  in git-commit(1)), and should skip the full stop
 	- the body should provide a meaningful commit message, which:
-	  . explains the problem the change tries to solve, iow, what
-	    is wrong with the current code without the change.
-	  . justifies the way the change solves the problem, iow, why
-	    the result with the change is better.
-	  . alternate solutions considered but discarded, if any.
+	  . explains the problem the change tries to solve, in other words,
+	    what is wrong with the current code without the change.
+	  . justifies the way the change solves the problem, in other words,
+	    why the result with the change is better.
+	  . mentions alternate solutions considered but discarded, if any.
 	- describe changes in imperative mood, e.g. "make xyzzy do frotz"
 	  instead of "[This patch] makes xyzzy do frotz" or "[I] changed
 	  xyzzy to do frotz", as if you are giving orders to the codebase
 	  to change its behaviour.
 	- try to make sure your explanation can be understood without
-	  external resources. Instead of giving a URL to a mailing list
-	  archive, summarize the relevant points of the discussion.
+	  external resources. Instead of providing only a URL to a
+	  mailing list archive, summarize the relevant points of the
+	  discussion.
 	- add a "Signed-off-by: Your Name <you@example.com>" line to the
 	  commit message (or just use the option "-s" when committing)
 	  to confirm that you agree to the Developer's Certificate of Origin
@@ -52,14 +53,14 @@ Checklist (and a short version for the impatient):
 
 Long version:
 
-I started reading over the SubmittingPatches document for Linux
+I started reading over the SubmittingPatches document for the Linux
 kernel, primarily because I wanted to have a document similar to
-it for the core GIT to make sure people understand what they are
-doing when they write "Signed-off-by" line.
+it for core GIT to make sure people understand what they are
+doing when they include a "Signed-off-by" line.
 
 But the patch submission requirements are a lot more relaxed
-here on the technical/contents front, because the core GIT is
-thousand times smaller ;-).  So here is only the relevant bits.
+here on the technical/contents front, because the core GIT is a
+thousand times smaller ;-).  So here are just the relevant bits.
 
 (0) Decide what to base your work on.
 
@@ -93,7 +94,7 @@ commit is the tip of the topic branch.
 (1) Make separate commits for logically separate changes.
 
 Unless your patch is really trivial, you should not be sending
-out a patch that was generated between your working tree and
+out a diff that was generated between your working tree and
 your commit head.  Instead, always make a commit with complete
 commit message and generate a series of patches from your
 repository.  It is a good discipline.
@@ -103,15 +104,8 @@ that people can judge if it is good thing to do, without reading
 the actual patch text to determine how well the code does what
 the explanation promises to do.
 
-If your description starts to get too long, that's a sign that you
-probably need to split up your commit to finer grained pieces.
-That being said, patches which plainly describe the things that
-help reviewers check the patch, and future maintainers understand
-the code, are the most beautiful patches.  Descriptions that summarise
-the point in the subject well, and describe the motivation for the
-change, the approach taken by the change, and if relevant how this
-differs substantially from the prior version, are all good things
-to have.
+If your commit message starts to get too long, that's a sign that you
+probably need to split your commit into finer grained pieces.
 
 Oh, another thing.  I am picky about whitespaces.  Make sure your
 changes do not trigger errors with the sample pre-commit hook shipped
-- 
1.7.2.3

  reply	other threads:[~2011-03-09 21:32 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-05  6:20 frustrated forensics: hard to find diff that undid a fix Adam Monsen
2011-03-05 10:00 ` Jonathan del Strother
2011-03-05 11:33 ` Jakub Narebski
2011-03-05 12:51   ` Jonathan Nieder
2011-03-05 13:48     ` Jeff King
2011-03-05 14:34   ` Adam Monsen
2011-03-05 19:56     ` [PATCH 0/2] improve combined diff documentation Adam Monsen
2011-03-05 19:56     ` [PATCH 1/2] documentation fix: git log -p does not imply -c Adam Monsen
2011-03-07  0:36       ` Junio C Hamano
2011-03-07 15:47         ` Jeff King
2011-03-07 18:37           ` Junio C Hamano
2011-03-07 19:12             ` Jeff King
2011-03-07 21:57               ` [PATCH v3] Documentation " Adam Monsen
2011-03-08  0:21                 ` Junio C Hamano
2011-03-08  0:49                   ` [PATCH v4] " Adam Monsen
2011-03-08 19:43                     ` Junio C Hamano
2011-03-08 20:46                       ` Adam Monsen
2011-03-09  0:58                         ` Junio C Hamano
2011-03-09 21:25                           ` Adam Monsen
2011-03-09 21:27                             ` Adam Monsen [this message]
2011-03-09 22:20                               ` [PATCH] SubmittingPatches: clean up commit message tips Junio C Hamano
2011-03-08 20:51                       ` [PATCH v5] diff format documentation: clarify --cc and -c Adam Monsen
2011-03-08 21:03                       ` [PATCH v6] " Adam Monsen
2011-03-08  1:19                   ` [PATCH v3] Documentation fix: git log -p does not imply -c Jeff King
2011-03-05 19:56     ` [PATCH 2/2] English grammar fixes for combined diff doc Adam Monsen
2011-03-05 14:29 ` frustrated forensics: hard to find diff that undid a fix Martin von Zweigbergk

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1299706069-5463-1-git-send-email-haircut@gmail.com \
    --to=haircut@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jnareb@gmail.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).