conventional-changelog-archived-repos / validate-commit-msg

DEPRECATED. Use https://github.com/marionebl/commitlint instead. githook to validate commit messages are up to standard
http://conventionalcommits.org/
MIT License
556 stars 101 forks source link

Make commit matching pattern more true to the spec #8

Closed Starefossen closed 8 years ago

Starefossen commented 8 years ago

This PR makes the commit message matching regular expression pattern more true to the spec by:

Before:

screen shot 2015-10-18 at 23 03 06

After:

screen shot 2016-02-05 at 12 40 22

Signed-off-by: Hans Kristian Flaatten hans@starefossen.com

cmalard commented 8 years ago

Hi Starefossen,

    var firstLine = match[1];
    var type = match[2];
    var scope = match[4];
    var subject = match[5];
kentcdodds commented 8 years ago

I need to give you an apology. For some reason, I want watching this repository so I want getting notifications about issues and pull requests. I am so incredibly sorry. Please forgive me. I really feel awful. I hope my unresponsiveness doesn't hinder you contributing more in the future.

As for your specific pull request, I'll give it a closer look soon.

kentcdodds commented 8 years ago

@Starefossen, thanks for the PR. Unfortunately, because I hadn't seen this until just this morning (sorry again), it no longer merges cleanly. Also, there have been new features added which you might consider taking advantage of. Do you mind looking at the current feature set and adjusting your PR to merge cleanly? Thanks!

Starefossen commented 8 years ago

All good @kentcdodds, I had totally forgot this PR as well :stuck_out_tongue_winking_eye:

Thank you for the suggestions @cmalard, I will have a look through this and make some changes and rebase on master to make this merge cleanly!

Since I proposed this PR I made a stand alone commit message ci to validate git commit messages: https://github.com/Starefossen/commit-message-ci

cmalard commented 8 years ago

You should also update the Readme ;-)

codecov-io commented 8 years ago

Current coverage is 100.00%

Merging #8 into master will not affect coverage as of a4756bc

@@            master      #8   diff @@
======================================
  Files            1       1       
  Stmts           53      53       
  Branches         0       0       
  Methods          0       0       
======================================
  Hit             53      53       
  Partial          0       0       
  Missed           0       0       

Review entire Coverage Diff as of a4756bc

Powered by Codecov. Updated on successful CI builds.

Starefossen commented 8 years ago

@cmalard what should I add/update the README with? @kentcdodds rebased on master and all tests are now passing!

kentcdodds commented 8 years ago

Hi! Still looking into this a bit, but tI just noticed that his is a breaking change.

This project uses semantic-release to automatically release a new version when code gets into master. To do this we need to follow the commit conventions (of which obviously you're well aware). Because this is a breaking change (no longer supports fixup), this needs to contain BREAKING CHANGE ${description} in the footer.

Could you please update your commit message? Thanks!

kentcdodds commented 8 years ago

Finished looking and I think I'm good to accept this. Just need the commit message to indicate the breaking change so the automatic release doesn't break people.

Starefossen commented 8 years ago

Added the breaking change notice to the message footer.

kentcdodds commented 8 years ago

Awesome! Thank you!

cmalard commented 8 years ago

It appears removing fixup! prevents using git commit --fixup <commit> http://fle.github.io/git-tip-keep-your-branch-clean-with-fixup-and-autosquash.html

Working on it :-)

kentcdodds commented 8 years ago

I'm more interested in following the spec personally. What follows the spec?

cmalard commented 8 years ago

This is not about the spec, this is about removing a feature of Git. Commits tagged with fixup! are not intended to be merged, see the previous link. What do you think about it ?