canonical-web-and-design / practices

Guides and principles from the web team at Canonical and Ubuntu
https://canonical-web-and-design.github.io/practices/
Other
45 stars 30 forks source link

Add commit message guideline document #138

Closed b-m-f closed 5 years ago

nottrobin commented 5 years ago

Thanks @b-m-f, great work

barrymcgee commented 5 years ago

I'm of the opinion Git history is very important and should be treated as documentation.

I had this drummed into me by a previous CTO I worked under and have been a convert to this idea ever since. I can't explain it any better than he can himself:

Video: https://vimeo.com/172882423 (9 mins) Slide breakdown: https://blog.mocoso.co.uk/talks/2015/01/12/telling-stories-through-your-commits/

bartaz commented 5 years ago

I'm of the opinion Git history is very important and should be treated as documentation.

It is important, and I fully agree that meaningful history is helpful. I use git blame quite often when trying to check when given change was introduced. Good commit message is first thing that gives some context. If commit allows to track down the PR it's even better (which is a GH feature for some time now). And we should at least try to educate ourselves in this regard (thanks for links @barrymcgee).

I guess what we are discussing here is how much we want to enforce it.

bartaz commented 5 years ago

One issue I have with unnecessary rebases is that it makes it harder to share given branch and update it locally.

Say I'm reviewing a branch and for that I pull it locally. Later it's rebased. I can't just pull it now as it will end up with duplicated conflicting changes. I need to reset and pull, or recreate the branch from scratch (unless there is some git magic command that does a 'pull after rebase').

nottrobin commented 5 years ago

@bartaz Yes, those are the salient questions. Personally, I feel quite strongly both that commits are important and people should care about them, and that we really shouldn't enforce them or block anything based on them.

But it seems this discussion is inexorably heading for another dev catch-up.

@bartaz: unless there is some git magic command that does a 'pull after rebase'

I'm having trouble reproducing the problem of not being able to git pull right now, but I think the command you're looking for may be git pull --force.

bartaz commented 5 years ago

@nottrobin Ah right, haven't done pull of rebased branch for a while because I just remembered I had issues with that. It doesn't break local branch, git will just refuse to do the pull if the pulled branch cannot be merged.

Didn't know you can --force pull. I need to check how that works. Does this override local branch with pulled one or does it try to merge them?

b-m-f commented 5 years ago

I have updated the doc. I hope it is now clearer that these should not be enforced rules, but rather a document to aid in finding a good way to write commit messages :) Taken from the desired state we would like to see in our git logs, even if this is certainly not always possible.

Here is also some further reading on the topic. By Tim Pope From the git website Rules in the Spring Framework

nottrobin commented 5 years ago

If everyone else is happy with these, :+1: from me

nottrobin commented 5 years ago

@hatched: It would also be nice to include a section on doing formatting-only changes in a separate commit or even a separate PR to allow code-only changes better visibility.

In general, it is preferred not to suggest new work on existing PRs. This is true of all projects, but especially of Practices. It should probably be explicitly stated in https://github.com/canonical-webteam/practices/blob/master/CONTRIBUTING.md, although it already carries this spirit.

You can easily file a new PR to add to this document once it is merged, and you can always suggest new work in issues.