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
557 stars 100 forks source link

Not throwing error when we ran from GIT GUI with husky #77

Closed krishna63 closed 7 years ago

krishna63 commented 7 years ago

Hi team,

I have installed husky and configured commitmsg hook to validate the commit message, when i try to commit form the git bash it is executing properly but when i try to commit from git GUI it is not throwing error, instead it is commiting.

Initially i was thinking that husky was skipping the hooks when i commit from git GUI. but when i configured another hook prepush, which executes jest (meaning it runs all the test cases) before doing any push. This hook is failing when i try to push from GUI as well, then i came to conclusion that it is problem with this module.

Kindly help me out

Garbee commented 7 years ago

Which GUI are you using? Some may force no hooks when committing which would bypass the test.

krishna63 commented 7 years ago

@Garbee : I am using GIT GUI on windows machine, so you are saying that GUI is skipping the hooks, if that is the case then, when i run some other command like jest which executes all the test case in my project. I am intentionally failing a particular test and then trying to commit.

So now the commit is aborted as the test case are failing. whereas when i use validate-commit-msg it is not happening eventhough my commit message is not inline with the specified rules.

kentcdodds commented 7 years ago

Ah, interesting. I've not seen this. Could you dig a little deeper to find the core problem and suggest a solution? Thanks!

pradeeptinku commented 7 years ago

@krishna63, @kentcdodds cli.js is looking for the commit message from '.git/COMMIT_EDITMSG' file irrespective of mode of committing the code (GIT BASH or GIT GUI or GITHUB) GIT GUI will be storing the commit message in '.git/GITGUI_EDITMSG' file and we are reading 'COMMIT_EDITMSG' file always for the commit message which is causing the issue.

if (process.argv.join('').indexOf('mocha') === -1) {
**  var commitMsgFileOrText = process.argv[2] || getGitFolder(),**
  //GIT GUI stores commit msg @GITGUI_EDITMSG file, 
  //if it doesn't exist then we are considering "/COMMIT_EDITMSG" file
  **  filename = 'GITGUI_EDITMSG';  **

  var validate = function (msg, isFile) {
    if (!validateMessage(msg)) {
      var incorrectLogFile = (
        isFile
       ** ? commitMsgFileOrText.replace(filename, 'logs/incorrect-commit-msgs')**
        : (getGitFolder() + '/logs/incorrect-commit-msgs')
      );

      fs.appendFile(incorrectLogFile, msg + '\n', function appendFile() {
        process.exit(1);
      });
    } else {
      process.exit(0);
    }
  };
 ** filename = (fs.existsSync(commitMsgFileOrText + filename)) ? filename : 'COMMIT_EDITMSG';
  commitMsgFileOrText += '/' + filename;**

I've modified the cli.js file as above to pick the commit msg appropriately (modified lines are in between double stars **) and tested with GIT GUI , GITHUB and GIT BASH and is working fine.

@Garbee , @kentcdodds , Please have a look and let me know your thoughts on this.

kentcdodds commented 7 years ago

Sounds great. Thanks for digging! I'd love to see a PR for this!

krishna63 commented 7 years ago

@pradeeptinku : thank you.

@kentcdodds : i have created the PR, have a look and process further. PR

maxcnunes commented 7 years ago

@Garbee You could close it since that PR has been merged already.

Garbee commented 7 years ago

Good spot. Thanks @maxcnunes.