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

Improve commit messages in 01-backup #70

Merged
merged 1 commit into from
Mar 19, 2015

Conversation

ethanwhite
Copy link
Contributor

We probably don't want to get into the details of good commit message style
in this lesson, but we should try to model it as best as possible. This updates
the commit messages to make them a little better/more standard.

This PR is currently missing updated html because until #68 goes in the diff shows far more change that have actually happened. I'll be happy to add the html changes after #68 is merged.

@rgaiacs
Copy link
Contributor

rgaiacs commented Mar 14, 2015

+1 to merge.

@wking
Copy link
Contributor

wking commented Mar 14, 2015

On Sat, Mar 14, 2015 at 01:06:49PM -0700, Ethan White wrote:

We probably don't want to get into the details of good commit
message style in this lesson, but we should try to model it as best
as possible. This updates the commit messages to make them a little
better/more standard.

This change:

  • The two moons may be a problem for Wolfman
  • Add concerns about effects of Mars' moons on Wolfman

is in the content of mars.txt, not the commit message.

You take “Thoughts about the climate” to “Add discussion of Mars'
climate”. I like yours more than the original, but how about “Discuss
Mars' climate for Mummy”? That will distinguish the commit from
subsequent commits which may also discuss the climate. If it was my
moon-base project, I'd like “mars: Discuss climate for Mummy”, but I
think the non-prefixed sentence may be less confusing for novices.

Regardless of the chosen climate commit message, I think this change
is good and moves us in the right direction (although I don't think we
want the content change for mars.txt).

This PR is currently missing updated html because until #68 goes in
the diff shows far more change that have actually happened. I'll be
happy to add the html changes after #68 is merged.

The maintainers are supposed to handle updating the HTML anyway, to
avoid conflicts in autogenerated-content from folks submitting HTML
updates in parallel PRs. See swcarpentry/DEPRECATED-lesson-template#159, and
point 3 of 1 which came from swcarpentry/DEPRECATED-lesson-template#170.

@iglpdc
Copy link
Contributor

iglpdc commented Mar 15, 2015

Yes, we don't need to change the contents of the file. @ethanwhite, could you fix that and then I'll merge?

By the way, I see that we don't give any explanation on how to write a commit message. Without going into much details, I think one good explanatory sentence (together with an entry in the lesson glossary) would be nice. I'm opening a new issue with this.

@daisieh
Copy link
Contributor

daisieh commented Mar 17, 2015

@ethanwhite: I merged the big reorg PR #58, so please refactor the change against that new file.

We should model good commit message style as best as possible.
This updates the commit messages to make them a little better/more standard.
@ethanwhite
Copy link
Contributor Author

Thanks for the great review @wking. I had accidentally find and replaced the wrong text for the 'moons' commit - thanks for catching that. I have now:

  1. Re-rolled this against Breaks up 01-backup.md in several topics #58
  2. Fixed the accidental replacement of the file content by changing the associated commit message instead
  3. Improved the climate commit message building on @wking's suggestion.

@wking
Copy link
Contributor

wking commented Mar 19, 2015 via email

daisieh added a commit that referenced this pull request Mar 19, 2015
Improve commit messages in 01-backup
@daisieh daisieh merged commit b422496 into swcarpentry:gh-pages Mar 19, 2015
PBarmby pushed a commit to PBarmby/git-novice that referenced this pull request Apr 17, 2015
zkamvar pushed a commit that referenced this pull request May 8, 2023
Improve commit messages in 01-backup
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

5 participants