6 ms·

A noble effort but in my experience these just get ignored. 90% of the PRs at my org have the empty, unfilled template text at the top of the description, a few

A noble effort but in my experience these just get ignored. 90% of the PRs at my org have the empty, unfilled template text at the top of the description, a few line breaks, and then the actual description underneath.

yakshaving_jgt 5y ago
That's unfortunate, and I think your experience reflects most places I've worked at too. It doesn't have to be that way though; technical leadership on a team can choose to care about this kind of thing and coach their colleagues into caring more about writing communicative commit messages. To any technical leaders reading this comment, I encourage you to choose to care about this :) Here's the post I always share on this: https://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html https://tbaggery.com/2008/04/19/a-note-about-git-commit-mess...
gaoshan 5y ago
We immediately kick them back if this is the case.
jasonpeacock 5y ago
1. Implement automation to flag and block/reject bad commit messages. 2. Educate and train ppl to include commit messages when they do code reviews or 3. give up and move on because your org doesn't care. Because this is a culture (not code) smell and changing culture is hard and needs support from the top.
- toomanybeersies 5y ago
  > Implement automation to flag and block/reject bad commit messages. They'll just use `git commit -n` to skip the pre-commit hook. And if it's verified in CI (e.g. with Danger), someone will just create a script to automate writing commit messages that pass the good commit message test.
  - jasonpeacock 5y ago
    And those are the people you fire because they refuse to follow policy and actively work to circumvent best practices and quality gates.
- hackerbrother 5y ago
  No way, (1) would annoy the heck out of people. Plus what if you want to just save off your current status and log off for the day..
  - jasonpeacock 5y ago
    Configure the enforcement only for `main` and other release/production branches, let people push whatever they want to their dev/feature branches. When those branches are attempted to be merged they'll fail due to having bad commits and require fixing.
  - toomanybeersies 5y ago
    > Plus what if you want to just save off your current status and log off for the day Strictly speaking, that's what git stash [1] is for. Not so ideal if you want to push your WIP changes to Github though. [1] https://git-scm.com/docs/git-stash https://git-scm.com/docs/git-stash
  - icedchai 5y ago
    Yes. In my experience, anything that prevents people from committing to local branches is incredibly annoying. You want people to commit frequently and “save their work.” Checks, quality barriers, etc. are fine at CI time.
  - blindmute 5y ago
    That's what git stash is for; you can even add a message, just like a commit, to the stash. A commit should always be in a good state, runnable, without lint or other errors.
cerved 5y ago
fwiw, the template text in this case are comments so that gets stripped
bpodgursky 5y ago
I always take care to delete the template text first.

yakshaving_jgt 5y ago

That's unfortunate, and I think your experience reflects most places I've worked at too. It doesn't have to be that way though; technical leadership on a team can choose to care about this kind of thing and coach their colleagues into caring more about writing communicative commit messages. To any technical leaders reading this comment, I encourage you to choose to care about this :) Here's the post I always share on this: https://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html https://tbaggery.com/2008/04/19/a-note-about-git-commit-mess...

gaoshan 5y ago

We immediately kick them back if this is the case.

jasonpeacock 5y ago

1. Implement automation to flag and block/reject bad commit messages. 2. Educate and train ppl to include commit messages when they do code reviews or 3. give up and move on because your org doesn't care. Because this is a culture (not code) smell and changing culture is hard and needs support from the top.

toomanybeersies 5y ago
> Implement automation to flag and block/reject bad commit messages. They'll just use `git commit -n` to skip the pre-commit hook. And if it's verified in CI (e.g. with Danger), someone will just create a script to automate writing commit messages that pass the good commit message test.
- jasonpeacock 5y ago
  And those are the people you fire because they refuse to follow policy and actively work to circumvent best practices and quality gates.
hackerbrother 5y ago
No way, (1) would annoy the heck out of people. Plus what if you want to just save off your current status and log off for the day..
- jasonpeacock 5y ago
  Configure the enforcement only for `main` and other release/production branches, let people push whatever they want to their dev/feature branches. When those branches are attempted to be merged they'll fail due to having bad commits and require fixing.
- toomanybeersies 5y ago
  > Plus what if you want to just save off your current status and log off for the day Strictly speaking, that's what git stash [1] is for. Not so ideal if you want to push your WIP changes to Github though. [1] https://git-scm.com/docs/git-stash https://git-scm.com/docs/git-stash
- icedchai 5y ago
  Yes. In my experience, anything that prevents people from committing to local branches is incredibly annoying. You want people to commit frequently and “save their work.” Checks, quality barriers, etc. are fine at CI time.
- blindmute 5y ago
  That's what git stash is for; you can even add a message, just like a commit, to the stash. A commit should always be in a good state, runnable, without lint or other errors.

cerved 5y ago

fwiw, the template text in this case are comments so that gets stripped

bpodgursky 5y ago

I always take care to delete the template text first.