README.md: adding a git style guide

[vbatts]

pulled from http://chris.beams.io/posts/git-commit/

Signed-off-by: Vincent Batts [email protected]

[wking]

On Wed, Sep 09, 2015 at 03:55:36PM -0700, Vincent Batts wrote:

pulled from http://chris.beams.io/posts/git-commit/

Maybe link there for folks who want more detail on or the motivation
behind a particular guideline?

[wking]

50 seems overly strict, especially if we want a prefix. Looking through our 1113 non-merge commits (with --format=%s) we only have 62% ≤50 chars, and but 78% are ≤ 70 chars. Of course some of those are because folks forgot the blank-line after the summary. But I still think a larger limit here would help encourage more explicit summaries.

[vbatts]

this is actually from git-commit(1) own guidelines. https://www.kernel.org/pub/software/scm/git/docs/git-commit.html

Though not required, it’s a good idea to begin the commit message with a single short (less than 50 character) line summarizing the change, followed by a blank line and then a more thorough description. The text up to the first blank line in a commit message is treated as the commit title, and that title is used throughout Git.

[wking]

On Wed, Sep 09, 2015 at 04:12:24PM -0700, Vincent Batts wrote:

+2. Limit the subject line to 50 characters

this is actually from git-commit(1) own guidelines.

Then I think everyone is just blindly copying it around ;). Looking
at the git.git history through v2.3.4 (git log --no-merges --format=%s
v2.3.4), we have 29853 commits, with 56% ≤ 50 chars and 94% ≤ 70
chars.

[wking]

And because it's been too long since I've busted out some Matplotlib, here's a histogram for git.git:

Generated with:

import sys
import matplotlib.pyplot
lines = sys.stdin.read().splitlines()
lengths = [len(line) for line in lines]
figure = matplotlib.pyplot.figure()
axes = figure.add_subplot(1, 1, 1)
axes.hist(lengths, 50)
axes.set_xlabel('commit message length (chars)')
axes.set_ylabel('counts in git.git through v2.3.4')
figure.savefig('x.png')

;).

[vbatts]

Jesus Trevor... I ....
On Sep 9, 2015 19:37, "W. Trevor King" [email protected] wrote:

In README.md
#168 (comment):

@@ -167,4 +169,16 @@ using your real name (sorry, no pseudonyms or anonymous contributions.)

You can add the sign off when creating the git commit via git commit -s.

+### Commit Style
+
+Simple house-keeping for clean git history.
+
+1. Separate subject from body with a blank line
+2. Limit the subject line to 50 characters

And because it's been too long since I've busted out some Matplotlib,
here's a histogram for git.git:

[image: x]
https://cloud.githubusercontent.com/assets/209920/9776781/d9e8bb6a-5710-11e5-9fd1-a7b62e54b677.png

Generated with:

import sysimport matplotlib.pyplot
lines = sys.stdin.read().splitlines()
lengths = [len(line) for line in lines]
figure = matplotlib.pyplot.figure()
axes = figure.add_subplot(1, 1, 1)
axes.hist(lengths, 50)
axes.set_xlabel('commit message length (chars)')
axes.set_ylabel('counts in git.git through v2.3.4')
figure.savefig('x.png')

;).

—
Reply to this email directly or view it on GitHub
https://github.com/opencontainers/specs/pull/168/files#r39110526.

[vbatts]

Regardless of the past commits, this would obviously be for future commits.
On Sep 9, 2015 19:38, "Vincent Batts" [email protected] wrote:

Jesus Trevor... I ....
On Sep 9, 2015 19:37, "W. Trevor King" [email protected] wrote:

In README.md
#168 (comment):

@@ -167,4 +169,16 @@ using your real name (sorry, no pseudonyms or anonymous contributions.)

You can add the sign off when creating the git commit via git commit -s.

+### Commit Style
+
+Simple house-keeping for clean git history.
+
+1. Separate subject from body with a blank line
+2. Limit the subject line to 50 characters

And because it's been too long since I've busted out some Matplotlib,
here's a histogram for git.git:

[image: x]
https://cloud.githubusercontent.com/assets/209920/9776781/d9e8bb6a-5710-11e5-9fd1-a7b62e54b677.png

Generated with:

import sysimport matplotlib.pyplot
lines = sys.stdin.read().splitlines()
lengths = [len(line) for line in lines]
figure = matplotlib.pyplot.figure()
axes = figure.add_subplot(1, 1, 1)
axes.hist(lengths, 50)
axes.set_xlabel('commit message length (chars)')
axes.set_ylabel('counts in git.git through v2.3.4')
figure.savefig('x.png')

;).

—
Reply to this email directly or view it on GitHub
https://github.com/opencontainers/specs/pull/168/files#r39110526.

[wking]

On Wed, Sep 09, 2015 at 04:38:49PM -0700, Vincent Batts wrote:

+2. Limit the subject line to 50 characters

Jesus Trevor... I ....

Graphs help people make decisions, right? ;)

[vbatts]

Or help give graphics for a slide deck...
On Sep 9, 2015 19:42, "W. Trevor King" [email protected] wrote:

In README.md
#168 (comment):

@@ -167,4 +169,16 @@ using your real name (sorry, no pseudonyms or anonymous contributions.)

You can add the sign off when creating the git commit via git commit -s.

+### Commit Style
+
+Simple house-keeping for clean git history.
+
+1. Separate subject from body with a blank line
+2. Limit the subject line to 50 characters

On Wed, Sep 09, 2015 at 04:38:49PM -0700, Vincent Batts wrote: > +2. Limit
the subject line to 50 characters Jesus Trevor... I ....
Graphs help people make decisions, right? ;)

—
Reply to this email directly or view it on GitHub
https://github.com/opencontainers/specs/pull/168/files#r39110896.

[wking]

On Wed, Sep 09, 2015 at 04:41:54PM -0700, Vincent Batts wrote:

+2. Limit the subject line to 50 characters

Regardless of the past commits, this would obviously be for future
commits.

This limit has been part of the Git docs for a while now, and they're
still not keeping under 50 (they also reference the 50-char soft limit
in Documentation/SubmittingPatches). I'd prefer just putting a
hard-limit at 70 chars in your validation tool (#167), but I'm ok
putting a limit of 50 in that validation tool (and still merging if
"commit summary is too long" is the only error). I just think wedging
useful summaries into 50-len(prefix) is going to be more trouble than
its worth.

[vbatts]

particular guideline?

[wking]

On Wed, Sep 09, 2015 at 03:55:36PM -0700, Vincent Batts wrote:

pulled from http://chris.beams.io/posts/git-commit/

And you seem to have left out what I think is the most important point
in Chris' post: what to put in the body 1. Was there a reason to
leave that out?

[vbatts]

Not a strong compulsion. I wonder if there is a better phrasing for that
bullet point.
On Sep 9, 2015 7:14 PM, "W. Trevor King" [email protected] wrote:

On Wed, Sep 09, 2015 at 03:55:36PM -0700, Vincent Batts wrote:

pulled from http://chris.beams.io/posts/git-commit/

And you seem to have left out what I think is the most important point
in Chris' post: what to put in the body 1. Was there a reason to
leave that out?

—
Reply to this email directly or view it on GitHub
#168 (comment).

[wking]

On Wed, Sep 09, 2015 at 04:08:56PM -0700, Vincent Batts wrote:

particular guideline?

In Chris' post, each bullet point links to a section explaining why
and how. The list your bringing in here is basically a table of
contents that references that more detailed information.

[wking]

On Wed, Sep 09, 2015 at 04:17:44PM -0700, Vincent Batts wrote:

Not a strong compulsion. I wonder if there is a better phrasing for
that bullet point.

I'm fairly comfortable with his phrasing. I usually talk about
“motivation” when I'm talking folks through good commit hygiene, but
his “why” is pretty much the same, and it's easier to type ;).

[vbatts]

updated

[vbatts]

And we can talk validation once the guide is in place. I'm not opposed to len() < 70

[wking]

The other entries are phrased to match an implicit “You should …”. For example, “You should separate…”. So maybe:

Prefix the subject (when possible) with a keyword scoping the change (e.g. “README: …”, “runtime: …”)

And maybe shift this up to sit with the other subject-related entries from earlier in the list.

[wking]

On Wed, Sep 09, 2015 at 08:08:29PM -0700, Vincent Batts wrote:

And we can talk validation once the guide is in place. I'm not
opposed to len() < 70

Ok.

I left some comments about minor nits with 3cbd157, but I don't think
it's anything that's worth holding up a merge here (i.e. I'm happy to
file nit-fix PRs after this lands if we don't want to finish polishing
all of that out here).

[laijs]

After I have read so many barren changelog from the docker.git, it is important that the following statement be included.

All the important/useful/essential information on the conversation of the PR should be copied to the changelog.

This statement makes 7. Use the body to explain what and why vs. how be taken into effect. The convenience of the GitHub entice every body write dryasdust changelogs.

[vbatts]

@laijs is a link to the conversation sufficient?

[laijs]

The link is inconvenient when git log to see what has been changed recently and why,
when git blame to find something, or when there is no networking.

I'm a 8years of linux-kernel developer of which community is highly value the changelog.

[wking]

On Thu, Sep 10, 2015 at 06:33:56AM -0700, Lai Jiangshan wrote:

All the important/useful/essential information on the conversation
of the PR should be copied to the changelog.

This is my personal preference too, but if we're documenting good
commit messages at the level of “remember to put a blank line between
the subject and the body”, I think it might be reaching too far for
now ;). Once we no longer need to talk about syntax, we can go back
with another pass and try to work on getting conversation summaries in
the commit message.

[vbatts]

@laijs PTAL

[laijs]

The change in the *.md looks good to me.

The rule 3/5 should apply for the sentence after the keyword of the subject,
so I think the subject of this change could be "README.md: Add a git style guide"

[vbatts]

@laijs good point. :-)
done

[laijs]

LGTM

[wking]

On Thu, Sep 10, 2015 at 05:41:52PM -0700, Lai Jiangshan wrote:

The rule 3/5 should apply for the sentence after the keyword of the
subject…

Other projects don't do this (e.g. git.git suggests lowecase after the
prefix 1), but I agree that “Capitalize the subject line” with the
“runtime: ...” example makes it pretty clear that it's the post-prefix
bit that gets Sentence cased.

… so I think the subject of this change could be "README.md: Add a
git style guide"

And I think this matches the style we're recommending.

[vbatts]

On Sep 11, 2015 12:14 AM, "W. Trevor King" [email protected] wrote:

On Thu, Sep 10, 2015 at 05:41:52PM -0700, Lai Jiangshan wrote:

The rule 3/5 should apply for the sentence after the keyword of the
subject…

Other projects don't do this (e.g. git.git suggests lowecase after the
prefix 1), but I agree that “Capitalize the subject line” with the
“runtime: ...” example makes it pretty clear that it's the post-prefix
bit that gets Sentence cased.

… so I think the subject of this change could be "README.md: Add a
git style guide"

And I think this matches the style we're recommending.

What are you saying? Yes or no to capitalization?

[wking]

On Fri, Sep 11, 2015 at 04:15:57AM -0700, Vincent Batts wrote:

What are you saying? Yes or no to capitalization?

I'm agnostic on capitalization, but with the current capitalization
rule, I think your 142c257 looks good. I was mostly saying that I
thought the current rule phrasing seemed clear enough to me, thanks to
the “runtime: ...” example ruling out the “capitalization means the
prefix” interpretation.

[vbatts]

k

On Fri, Sep 11, 2015 at 10:05 AM, W. Trevor King [email protected]
wrote:

On Fri, Sep 11, 2015 at 04:15:57AM -0700, Vincent Batts wrote:

What are you saying? Yes or no to capitalization?

I'm agnostic on capitalization, but with the current capitalization
rule, I think your 142c257 looks good. I was mostly saying that I
thought the current rule phrasing seemed clear enough to me, thanks to
the “runtime: ...” example ruling out the “capitalization means the
prefix” interpretation.

—
Reply to this email directly or view it on GitHub
#168 (comment).

[crosbymichael]

LGTM

[mrunalp]

LGTM