-
Notifications
You must be signed in to change notification settings - Fork 602
README.md: adding a git style guide #168
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -115,6 +115,8 @@ To keep consistency throughout the Markdown files in the Open Container spec all | |
| This fixes two things: it makes diffing easier with git and it resolves fights about line wrapping length. | ||
| For example, this paragraph will span three lines in the Markdown source. | ||
|
|
||
| ## Git commit | ||
|
|
||
| ### Sign your work | ||
|
|
||
| The sign-off is a simple line at the end of the explanation for the patch, which certifies that you wrote it or otherwise have the right to pass it on as an open-source patch. | ||
|
|
@@ -167,4 +169,19 @@ 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. | ||
| Read more on [How to Write a Git Commit Message](http://chris.beams.io/posts/git-commit/) or the Discussion section of [`git-commit(1)`](http://git-scm.com/docs/git-commit). | ||
|
|
||
| 1. Separate the subject from body with a blank line | ||
| 2. Limit the subject line to 50 characters | ||
| 3. Capitalize the subject line | ||
| 4. Do not end the subject line with a period | ||
| 5. Use the imperative mood in the subject line | ||
| 6. Wrap the body at 72 characters | ||
| 7. Use the body to explain what and why vs. how | ||
| * If there was important/useful/essential conversation or information, copy or include a reference | ||
| 8. When possible, one keyword to scope the change in the subject (i.e. "README: ...", "runtime: ...") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The other entries are phrased to match an implicit “You should …”. For example, “You should separate…”. So maybe:
And maybe shift this up to sit with the other subject-related entries from earlier in the list. |
||
|
|
||
| [BlueJeans]: https://bluejeans.com/1771332256/ | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is actually from
git-commit(1)own guidelines. https://www.kernel.org/pub/software/scm/git/docs/git-commit.htmlThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On Wed, Sep 09, 2015 at 04:12:24PM -0700, Vincent Batts wrote:
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And because it's been too long since I've busted out some Matplotlib, here's a histogram for git.git:
Generated with:
;).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Jesus Trevor... I ....
On Sep 9, 2015 19:37, "W. Trevor King" [email protected] wrote:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regardless of the past commits, this would obviously be for future commits.
On Sep 9, 2015 19:38, "Vincent Batts" [email protected] wrote:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On Wed, Sep 09, 2015 at 04:38:49PM -0700, Vincent Batts wrote:
Graphs help people make decisions, right? ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or help give graphics for a slide deck...
On Sep 9, 2015 19:42, "W. Trevor King" [email protected] wrote:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On Wed, Sep 09, 2015 at 04:41:54PM -0700, Vincent Batts wrote:
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.