Skip to content
This repository has been archived by the owner on Jan 29, 2021. It is now read-only.

Commit message format #42

Open
ErikSchierboom opened this issue Aug 16, 2018 · 16 comments
Open

Commit message format #42

ErikSchierboom opened this issue Aug 16, 2018 · 16 comments

Comments

@ErikSchierboom
Copy link
Member

ErikSchierboom commented Aug 16, 2018

In the problem-specifications repository, we put a lot of effort in ensuring that the commit messages are very descriptive. Most commit messages use a format of: "[exercise name]: [change made]". This makes for quite a descriptive commit history. Maybe we should apply a similar naming policy here? For example something like: "[track name]: [change made]"? An example commit message would then be: "csharp: Add mentoring notes for anagram exercise". An alternative could be "[track name]/[exercise]: [change made]", which could look like this: "csharp/anagram: Add mentoring notes". I think I like the last one best.

@frerich
Copy link
Contributor

frerich commented Aug 16, 2018

I think this is a no-brainer. 👍 from me!

@iHiD
Copy link
Member

iHiD commented Aug 16, 2018

I'm really split on this. On one side I really like good commit messages. On the other side, the fact that I really want to eventually have dozens of PRs/commits a day in here makes me worry that having too strict rules will slow people down. In the website-copy repo we've already had 350 PRs (1/3 of the all-time for problem-specifications). I worry if we had had rules there that we enforced, we'd still have a huge backlog asking for amendments. Or we push the responsibility to the PR merger to reword the commit message?

What's the specific advantage in this scenario for good commit messages?

@frerich
Copy link
Contributor

frerich commented Aug 16, 2018

I suppose the risk of slowing PRs down due to rules regarding the format of commit messages could be assessed by looking at the problem-specifications repository. Someone who's involved in merging PRs could probably give a representative estimate on how much of a pain such a rule is in practice.

@ErikSchierboom
Copy link
Member Author

I see your point, as we really want to have people create these commits. Perhaps we should not be very strict on enforcing the rules, but some guidance could be given when merging a PR. Something like: "Maybe perhaps in a future PR format your commit message like ..."

There is no real, huge benefit of having good commit messages, except for an easier to browse commit history and just good source control hygiene. Having commits like "Created folder", "Changes after review" and "Create README.md" just feels a bit wrong to me. But maybe that's just me!

p.s. I already saw that I didn't communicate my naming policy well enough, as I didn't intend for the brackets to be included :D

@ErikSchierboom
Copy link
Member Author

I suppose the risk of slowing PRs down due to rules regarding the format of commit messages could be assessed by looking at the problem-specifications repository. Someone who's involved in merging PRs could probably give a representative estimate on how much of a pain such a rule is in practice.

I do merge PR's in that repository and I rarely find that this is a problem.

@frerich
Copy link
Contributor

frerich commented Aug 16, 2018

Arguing for rules to adhere to in order to get nice commit logs is always a bit of an uphill battle; the benefit of being able to identify relevant commits e.g. purely by reading the output of git shortlog only actually becomes apparent when you require that ability. Which may be never -- so the decision is a matter of risk management.

@ErikSchierboom
Copy link
Member Author

ErikSchierboom commented Aug 16, 2018

@iHiD I've updated by starting comment with my new favorite format "[track name]/[exercise]: [change made]". A sample commit message would be "csharp/anagram: Add mentoring notes".

@kytrinyx What is your opinion here?

Arguing for rules to adhere to in order to get nice commit logs is always a bit of an uphill battle; the benefit of being able to identify relevant commits e.g. purely by reading the output of git shortlog only actually becomes apparent when you require that ability. Which may be never -- so the decision is a matter of risk management.

This is very much true. It just may be the OCD side of me that made me create this issue.

@iHiD
Copy link
Member

iHiD commented Aug 16, 2018

I like this issue :)

So I'm a stickler for good git histories with code. Because code has unexpected side effects and debugging etc, and git is a super-important part of that. Because this is just stand-alone text, I don't have the same concerns in this repo. That doesn't mean we shouldn't try and adhere to something but it means I care a lot less about it than I would in another repo.

I also find that when editing text in the GitHub UI I write good PR titles but bad commit messages and because there's not an easy way (that I know of (and therefore that probably most other people know of)) to change it, that we're going to see lots of bad messages.

I suggest:

  • We as repo-maintainers adhere to it (without square brackets in future ;))
  • PR mergers should squash/rename commits in the UI to follow the pattern

I did suggest (and then delete) that we have a CONTRIBUTING.md that asks people to do it, but that'll be after-the-fact of the commit anyway.

@ErikSchierboom
Copy link
Member Author

I fully agree with your comments. Thanks for taking the time to eloquently explain your thought! Should this issue be closed or should we keep it open until we have a CONTRIBUTING.md file?

@iHiD
Copy link
Member

iHiD commented Aug 16, 2018

Let's leave it open and then once one of us PRs a change to the README and makes a CONTRIBUTING.md we can close it.

@kytrinyx
Copy link
Member

I rely heavily on the git history for debugging and gaining an understanding of what has gone before (even when there are no bugs).

I'm very much in favor of having good commit messages, but (a) it's really hard to enforce, (b) not all the commits will be about exercises, and (c) I worry that making the people who merge things clean up commits will add a burden that makes it more likely for things to remain open.

I'm OK with trying the above suggestions and seeing how we fare, though!

@ErikSchierboom
Copy link
Member Author

Let's just make it a best effort. If we can, we should try to use well-named commit messages preferrably adhering to the mentioned format, but it should not get in the way.

@iHiD
Copy link
Member

iHiD commented Aug 17, 2018

I tried to make a CONTRIBUTING.md but I'm not sure what to put in there. Specifically because it won't show up until after someone has made the commit and is now making the Pull Request. Help/thoughts appreciated :)

@hilary
Copy link
Contributor

hilary commented Aug 17, 2018

By all means write the CONTRIBUTING.md! Just include a link to a well written walk-through for git rebase -i 😄

@iHiD
Copy link
Member

iHiD commented Aug 17, 2018

:) I know you're joking, but on a serious note, I want this repo and website-copy to both be contributable to without having Git installed on ones computer

@NobbZ
Copy link
Member

NobbZ commented Aug 24, 2018

I do not think, that we need a certain format for commit-messages, but I'd be very happy if PR titles would include track and exercise.

eg: “elixir/rna-transcription: add readme” or "scala/hello-world: update example”.

This repository deals with a lot of tracks, that share exercises, therefor I think it is confusing to have couple of PRs dealing with “RNA transcription” and spelling variations which touch files in different corners of the repository, which still all are named README.md

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants