5 ms·

Head of Engineering with 15 years of experience here. If found that when we starting using Github flow, with pull-requests and etc, commit messages stopped rea

Head of Engineering with 15 years of experience here. If found that when we starting using Github flow, with pull-requests and etc, commit messages stopped really matter, in favour of PR descriptions. Plus when you use "squash" strategy, and use PR description as commit message, history looks great. When you look at commit history through Github tooling, all PR ids turned into links, and it is very easy watch for the history. Additionally you have a good integration with Github features, like discussions (which sometimes as important as description itself). Yes it is a vendor lock-in, but lets be frank, Github most likely will survive your project.

gregmac 6y ago
The commit message with a good PR description is okay, but for any sizable code change still doesn't provide full context for an individual piece of code (git blame). Either the PR description is massive -- in which case it's hard to read and tell what applies where -- or it glosses over the detail that's necessary. The thing is this isn't obvious why it's useful until you don't have it. I don't do it often, but the things that make me go back looking through git blame and history are trying to identify where hardcoded constants came from, or when (time or version) a particular bug was first introduced and why. This is only necessary when there aren't decent comments in the code already, and in my experience, the types of developers that don't put in these comments are also the ones that don't make highly detailed PR descriptions. Good commit messages is also a hurdle, but an easier one to get over than highly-detailed PR descriptions. On top of that you get all the other significant downsides of a squash merge strategy (eg: not being able to tell the difference between a local branch being merged or not pushed; confusing merge conflicts if you ever merge branch->branch before merging one to master). As an aside, IMHO, "clean history" is not at all a useful feature. Only developers are looking at git log to begin with, and they are quite capable of using `git log --merges` (or equivalent GUI operation). I guess the most useful thing is to make a CHANGES file (release notes) -- but honestly, the wording is different anyway (eg, "Fixed several minor UI issues" is adequate for release notes, but that might be across several PRs that contain more detail like "Fixed button alignment in modal dialogs", "Upgraded bootstrap to version x.y.z" etc). Go back and look at your PR descriptions from a year ago and see if you can make sense of them -- I bet the context of many will be lost.
geewee 6y ago
Azure devops also allows you to squash commits, I don't think this is Github specific. I do however agree with squashing commits in PRs being really nice (unless you want to branch out from a branch that gets squashed, then it sucks)
- u801e 6y ago
  If you squash commits in a PR down to a single commit, then doesn't that result in a commit that makes a lot of changes that makes it harder to pinpoint a bug or revert without conficts? A lot of features take more than a single logical commit to implement, so it makes sense to have multiple logical commits associated with a feature update. Squashing them down to a single commit makes it harder to review in my opinion.
jacquesm 6y ago
What's wrong with placing that documentation right where it belongs?: In the code.
- pcl 6y ago
  I consider the version control history to be part of the code. I put docs into the source tree (sometimes in the form of comments; sometimes in dedicated doc files) for things that are suited to live next to the code, but often my commit messages contain more discussion of what used to be and why I chose a certain implementation approach. I generally think that documentation in code should describe what the code does and why, and commit messages should describe why critical choices were made and how the new approach differs from previous behavior. This assumes a good version control system that follows history across renames and moves etc., of course.
  - jacquesm 6y ago
    Code tends to live longer than the VCS that stores it. I've seen more than one VCS migration that ended up losing a lot of the history of how things got to be where they are today.
    - pcl 6y ago
      I guess I’m an optimist on that front. I’ve lost history from CVS and SCCS, but not from SVN, Perforce, Git or Mercurial, including a number of Git module extractions. I think that history-losing version control migrations are a thing of the past.
      - jacquesm 6y ago
        Check out a 'zip' file to cross port to a system that does not support your VCS of choice (many embedded systems, for instance) and poof half your docs are gone... VCS should store the code, commit messages should aid in bi-secting but should not explain too much other than to clearly document what was changed in that commit with reference to a particular ticket if available. That way you keep the meta stuff in one place and all the action where the code itself lives, and where you are most likely to need it. I'm a big fan of literate programming, and VCS is not an integral part of that.
saurik 6y ago
You say that, and yet I have seen so many projects large and small transition between various code storage platforms and lose history of random ancillary conversations, due to how all of these tend to be locked up walled gardens of tools that prevent a useful "import" :/.

gregmac 6y ago

The commit message with a good PR description is okay, but for any sizable code change still doesn't provide full context for an individual piece of code (git blame). Either the PR description is massive -- in which case it's hard to read and tell what applies where -- or it glosses over the detail that's necessary. The thing is this isn't obvious why it's useful until you don't have it. I don't do it often, but the things that make me go back looking through git blame and history are trying to identify where hardcoded constants came from, or when (time or version) a particular bug was first introduced and why. This is only necessary when there aren't decent comments in the code already, and in my experience, the types of developers that don't put in these comments are also the ones that don't make highly detailed PR descriptions. Good commit messages is also a hurdle, but an easier one to get over than highly-detailed PR descriptions. On top of that you get all the other significant downsides of a squash merge strategy (eg: not being able to tell the difference between a local branch being merged or not pushed; confusing merge conflicts if you ever merge branch->branch before merging one to master). As an aside, IMHO, "clean history" is not at all a useful feature. Only developers are looking at git log to begin with, and they are quite capable of using `git log --merges` (or equivalent GUI operation). I guess the most useful thing is to make a CHANGES file (release notes) -- but honestly, the wording is different anyway (eg, "Fixed several minor UI issues" is adequate for release notes, but that might be across several PRs that contain more detail like "Fixed button alignment in modal dialogs", "Upgraded bootstrap to version x.y.z" etc). Go back and look at your PR descriptions from a year ago and see if you can make sense of them -- I bet the context of many will be lost.

geewee 6y ago

Azure devops also allows you to squash commits, I don't think this is Github specific. I do however agree with squashing commits in PRs being really nice (unless you want to branch out from a branch that gets squashed, then it sucks)

u801e 6y ago
If you squash commits in a PR down to a single commit, then doesn't that result in a commit that makes a lot of changes that makes it harder to pinpoint a bug or revert without conficts? A lot of features take more than a single logical commit to implement, so it makes sense to have multiple logical commits associated with a feature update. Squashing them down to a single commit makes it harder to review in my opinion.

jacquesm 6y ago

What's wrong with placing that documentation right where it belongs?: In the code.

pcl 6y ago
I consider the version control history to be part of the code. I put docs into the source tree (sometimes in the form of comments; sometimes in dedicated doc files) for things that are suited to live next to the code, but often my commit messages contain more discussion of what used to be and why I chose a certain implementation approach. I generally think that documentation in code should describe what the code does and why, and commit messages should describe why critical choices were made and how the new approach differs from previous behavior. This assumes a good version control system that follows history across renames and moves etc., of course.
- jacquesm 6y ago
  Code tends to live longer than the VCS that stores it. I've seen more than one VCS migration that ended up losing a lot of the history of how things got to be where they are today.
  - pcl 6y ago
    I guess I’m an optimist on that front. I’ve lost history from CVS and SCCS, but not from SVN, Perforce, Git or Mercurial, including a number of Git module extractions. I think that history-losing version control migrations are a thing of the past.
    - jacquesm 6y ago
      Check out a 'zip' file to cross port to a system that does not support your VCS of choice (many embedded systems, for instance) and poof half your docs are gone... VCS should store the code, commit messages should aid in bi-secting but should not explain too much other than to clearly document what was changed in that commit with reference to a particular ticket if available. That way you keep the meta stuff in one place and all the action where the code itself lives, and where you are most likely to need it. I'm a big fan of literate programming, and VCS is not an integral part of that.

saurik 6y ago

You say that, and yet I have seen so many projects large and small transition between various code storage platforms and lose history of random ancillary conversations, due to how all of these tend to be locked up walled gardens of tools that prevent a useful "import" :/.