mediamonks / frontend-coding-standards

Media.Monks - Frontend Coding Standards
60 stars 23 forks source link

Standardise commit messages #16

Open evertmonk opened 4 years ago

evertmonk commented 4 years ago

I'm not sure if this is something for the coding standards but I like to have a look at this. I think it's a good idea to put some sort of standardisation on commit messages.

I personally try to follow the angular way of writing commit messages, which can be read here Angular Commit Messages

The commit message looks like the following: <type>(<scope>): <subject>

Pros

Cons

I would like to hear everybody's thoughts on this.

giuseppesalvo commented 4 years ago

I totally agree. I've also started using this convention some months ago, and I feel that it really improves the readability of commits. https://www.conventionalcommits.org/en/v1.0.0/

We could use husky and commitlint to enforce the message format: https://github.com/conventional-changelog/commitlint#getting-started

ThaNarie commented 4 years ago

I always follow this one: https://chris.beams.io/posts/git-commit/

Depending on the project, with 1 change - include the ticket number in the subject line. This makes the commit log easier to scan related to specific tickets.

Regarding your link The <type>(<scope>) part could be useful, but I think that's more suited towards long-running projects with defined types and scopes that need to be set up, so everyone uses the same ones. For shorter projects, it might be work well since it's not defined.

The thing they have in common should be followed anyway, since they are a big improvement over what some people come up with :D

We could see if we can pick an existing standard from https://github.com/commitizen/cz-cli#adapters (or create our own template(s) to better integrate with the tools we use) - but should also look how people commit using their editors (webstorm/vscode) or git tools (sourcetree, tortoisegit, etc)

ThijsTyZ commented 4 years ago

I like the idea of standardizing commit messages. However writing a commit message like this doesn't mean it's good or helpful. For me knowing why something is changed is more important than seeing what is changed. Your example change primary color to value #ff0000 is an example of a commit message that does not give me any information why it is changed. If you look at the changes it's clear that you changed a color. But I want to know why that color was change. So you should add something like: because this will make the text better readable for xxx or because of ticket #123 or because of latest design changes of xxx. And if you can link the commit message to a ticket, you should always do that. So commit message ready #123 is much better (to me) then change primary color to value #ff0000.

evertmonk commented 4 years ago

Yeah that makes sense.

Ticket numbers are indeed super helpful. Maybe the explanation of why something was done a certain way can be added to the body of the commit? I'm not sure if it's something that I would add in the title.

I use titles more of a summary of all the changes that have occurred and if I want to know more about a specific commit I will check the description.

The only thing is (myself included) most of the time people only write a commit title so it might get some getting use to to also write a description

ThaNarie commented 4 years ago

For me knowing why something is changed is more important than seeing what is changed.

You need both:

  1. title - the change itself (so it's scannable in the log) - you always need this.
  2. body - more info about the change, including the why if it's not obvious from the subject or clearly explained in the linked ticket.
leroykorterink commented 3 years ago

Regarding enforcing commit message style. Husky is usually installed in new projects, we can create a pre-commit hook to enforce a certain commit message styling.

"husky": {
  "hooks": {
    "pre-commit": "node ./path-to-script/pre-commit.js",
    // Or as an export of package
    "pre-commit": "frontend-coding-standards/git-hooks/pre-commit",
  }
}

The script doesn't have to be too complex. This is something I've created for a project I've been working on. It checks if the ticket matches a pattern so that we can find it in Jira.

const fs = require('fs');
const path = require('path');
const chalk = require('chalk');

const commitMessage = fs.readFileSync(
  path.join(__dirname, relativePathToRootOfRepository, '.git/COMMIT_EDITMSG'),
  'utf8',
);

const isMergeCommit = /^Merge branch/.test(commitMessage);

if (isMergeCommit) {
  process.exit(0);
  return;
}

const startsWithTicketId = /^MYPROJECT-\d{1,} /.test(commitMessage);

if (!startsWithTicketId) {
  console.error(
    `\n${chalk.red('Commit message is not starting with a issue id')}\n\n${chalk.cyan(
      'Example:\nMYPROJECT-123 This is my commit title',
    )}\n`,
  );
  process.exit(1);
  return;
}

process.exit(0);

Conveniently bitbucket is also linked in that project, this makes tracking changes a lot easier us and PMs. I've worked on projects where a hook also checks if the ticket id actually exists by eg. connecting to the Jira API (https://jira.atlassian.net/rest/api/latest/issue/MYPROJECT-123)