Skip to content
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

Update 03-changes.md #78

Merged
merged 4 commits into from
Mar 25, 2015
Merged

Conversation

janepipistrelle
Copy link
Contributor

Here's my stab at adding a sentence on composing commit messages as per issue #71. I haven't included the Tim Pope link (http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html) here as I thought it would be better to put that in a glossary and link to it, given that the lesson time is stated at 20 minutes, but I have tried to summarise his key points.

@wking
Copy link
Contributor

wking commented Mar 20, 2015

On Fri, Mar 20, 2015 at 09:27:32AM -0700, janepipistrelle wrote:

Here's my stab at adding a sentence on composing commit messages as
per issue #71.

Looks pretty good to me. To emphasize the choice for more detail, may
split into two sentences with something like:

Good commit messages start with a brief (<50 characters) summary of
changes made in the commit. If you want to go into more detail, add
a blank line between the summary line and your additional notes.

I haven't included the Tim Pope link
(http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html)
here as I thought it would be better to put that in a glossary and
link to it, given that the lesson time is stated at 20 minutes, but
I have tried to summarise his key points.

I'd still link from here with “Good commit messages” as the anchor
text. If/when we add a glossary entry we can replace the link target
with the URL for the glossary entry. I don't understand why
teaching-time impacts whether or not to place a link, but I think the
more reference links we can scatter through the lesson, the better.
It just makes it easier for folks reading through the lesson text
(instructors and self-teaching students) to get more information or
find out why we're saying what we're saying.

If you do end up adding a link, I'd recommend reference-style links
1, because it makes it easy to update link targets like this without
needing to tweak the anchor text. So I'd go with:

Good commit messages start…

and then at the end of the file:

@janepipistrelle
Copy link
Contributor Author

Thanks for the comments @wking I think those are really nice touches and I'll add them.

@daisieh
Copy link
Contributor

daisieh commented Mar 22, 2015

This is short and sweet; I like it! Merge?

@wking
Copy link
Contributor

wking commented Mar 22, 2015

On Sat, Mar 21, 2015 at 09:29:58PM -0700, Daisie Huang wrote:

This is short and sweet; I like it! Merge?

It looks good to me.

@iglpdc
Copy link
Contributor

iglpdc commented Mar 22, 2015

Thanks, @janepipistrelle! Maybe it sounds a little better to me "short, descriptive message" instead of your "short, unique message", but I'm happy with the current version too.

@wking
Copy link
Contributor

wking commented Mar 22, 2015

On Sun, Mar 22, 2015 at 07:16:49AM -0700, Ivan Gonzalez wrote:

Maybe it sounds a little better to me "short, descriptive message"
instead of your "short, unique message", but I'm happy with the
current version too.

Hmm, “unique” is probably too strong, but I think it's good for folks
to consider how the summary compares with other potential commit
messages. To pick on a few recent git-novice commits at random, these
are summaries that I think could have been more precise:

  • fc197f3 (Minor language simplification, 2015-03-18)
  • 7437af1 (remove old versions, 2015-03-17)

I'd have gone with summaries like:

  • 03-changes: Remove 'posterity' and other language cleanups
  • Remove stale HTML after splitting 01-backup.md

“descriptive” is good, but I'd like a word that also implies that
comparison. Maybe “a short, distinctive summary”? Or “specific”?

@iglpdc
Copy link
Contributor

iglpdc commented Mar 22, 2015

Yep, any of those "descriptive, distinctive, specific" is a little bit better than unique.

@janepipistrelle, your pick: I'll merge any of these or something else you come up with.

@iglpdc iglpdc self-assigned this Mar 22, 2015
@janepipistrelle
Copy link
Contributor Author

any of those are fine with me. I picked unique to emphasise not writing the same thing for each commit message (ie "fixed bug").

@daisieh
Copy link
Contributor

daisieh commented Mar 23, 2015

I prefer "descriptive," if possible: uniqueness and specificity are nice-to-have features of a good commit message, but really, the only necessary criterion is "descriptive." @janepipistrelle, if you could push that change up to your branch, I'll gladly merge the PR.

@wking
Copy link
Contributor

wking commented Mar 23, 2015

On Mon, Mar 23, 2015 at 03:22:07PM -0700, Daisie Huang wrote:

I prefer "descriptive," if possible: uniqueness and specificity are
nice-to-have features of a good commit message, but really, the only
necessary criterion is "descriptive."

I wanted a word that implied “descriptive in the context of all the
other commit messages for this repository”. I see lots of commit
messages that accurately describe the change, but without enough
specificity to figure out what that commit's actually doing
(e.g. which typo is “typo fix” fixing?).

And I think the whole point of this is to describe the nice-to-have
features ;). You can have a blank commit message (with
--allow-empty-message), so none of this is actually necessary. We
just need to figure out how many nice-to-have features we can pack
into a clear sentence or two ;).

All of that said, You and Ivan are the maintainers, so it's your call.
I'm just trying to explain the choice I'd make and why. If you hear
my arguments and “descriptive” speaks to you most strongly, then let's
go with that.

@daisieh
Copy link
Contributor

daisieh commented Mar 23, 2015

Commit messages are so short that, IMO, it's nearly impossible to really contextually describe a change, so it's more of a goal to strive for than an actual goal. In practice, probably 80% of commit messages are things like "typo fix" or, for me, "test." I'd rather people commit things frequently with bad messages than put off committing things because writing a good message was hard. Personally, I'd prefer to have the change be really small and obvious in the diff directly, with a short commit message that provides a hint, than have a really good commit message that actually obscures a whole bunch of stuff in the diff.

@wking
Copy link
Contributor

wking commented Mar 23, 2015

On Mon, Mar 23, 2015 at 04:27:18PM -0700, Daisie Huang wrote:

Commit messages are so short that, IMO, it's nearly impossible to
really contextually describe a change…

I don't buy it ;). Or at least, I see no reason to give up before
trying ;).

In practice, probably 80% of commit messages are things like "typo
fix" or, for me, "test."

For typos, I usually use:

{path-to-tweaked-file}: Fix '{old}' -> '{new}' typo

For example, in a work repository, I have 1.

I'd rather people commit things frequently with bad messages than
put off committing things because writing a good message was
hard.

Absolutely. But I don't think using “specific” here will scare them
off ;).

Personally, I'd prefer to have the change be really small and
obvious in the diff directly, with a short commit message that
provides a hint, than have a really good commit message that
actually obscures a whole bunch of stuff in the diff.

I agree. But I really prefer small, obvious changes with really good
commit messages ;). I see no reason to avoid pushing gently for both.

@iglpdc
Copy link
Contributor

iglpdc commented Mar 24, 2015

For typos, I usually use:

{path-to-tweaked-file}: Fix '{old}' -> '{new}' typo

Yes, I've trying to do that after realizing that 80% of my commits messages say "minor fix", "fix typo"... I agree with @wking that good messages are really important, even if it takes a lot of discipline.

But one thing, we don't have to use only two words only :-)

We can say "short, descriptive, and specific" and cover both views.

@wking
Copy link
Contributor

wking commented Mar 24, 2015

On Mon, Mar 23, 2015 at 06:17:04PM -0700, Ivan Gonzalez wrote:

We can say "short, descriptive, and specific" and cover both views.

You rebel ;). I feel silly, and the three-word approach sounds good
to me :).

in sentence describing good commit messages
@iglpdc
Copy link
Contributor

iglpdc commented Mar 25, 2015

Great! I'm merging this.

Thanks @janepipistrelle for the PR and everyone for a great discussion.

iglpdc added a commit that referenced this pull request Mar 25, 2015
Add a brief explanation of how write good commit messages in 03-changes.md.
@iglpdc iglpdc merged commit f2bc62b into swcarpentry:gh-pages Mar 25, 2015
zkamvar pushed a commit that referenced this pull request May 8, 2023
Add a brief explanation of how write good commit messages in 03-changes.md.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants