Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove grunt githook and replace with package.json's postinstall script #3606

Merged

Conversation

mlasak
Copy link
Contributor

@mlasak mlasak commented Apr 13, 2021

The hook is installed using a node script

Pulling this functionality out of #3253
As discussed in #3596

@mlasak
Copy link
Contributor Author

mlasak commented Apr 13, 2021

@dsilhavy wdyt?

@dsilhavy dsilhavy added this to the 4.0.0 milestone Apr 13, 2021
@dsilhavy dsilhavy self-requested a review April 13, 2021 13:00
@dsilhavy dsilhavy merged commit 659f74b into Dash-Industry-Forum:development Apr 13, 2021
@bbert
Copy link
Contributor

bbert commented Apr 14, 2021

@mlasak have you tested on Windows? It seems there is an issue on Windows with the folder path

@mlasak
Copy link
Contributor Author

mlasak commented Apr 15, 2021

sorry that you seeing an issue here. But yes, we have tested on Mac and Win, even did some changes to make it work on Win. Could you post the problem you see?
@dsilhavy fyi

@dsilhavy
Copy link
Collaborator

@bbert I saw an issue with folders on Windows first as well. Basically an error was triggered when the pre-commit folder already existed. @mlasak made some changes and it solved the problem. Did you check with the latest changes in this branch?

@bbert
Copy link
Contributor

bbert commented Apr 15, 2021

I tested once PR has been merged.
There is only one commit in this branch, isn't it?

@mlasak
Copy link
Contributor Author

mlasak commented Apr 15, 2021

Yes, only one single commit and exactly this one that was merged. If you post the error message we could maybe better understand what's going wrong.

@bbert
Copy link
Contributor

bbert commented Apr 15, 2021

{ Error: spawn C:\Windows\system32\cmd.exe ENOENT
    at Process.ChildProcess._handle.onexit (internal/child_process.js:240:19)
    at onErrorNT (internal/child_process.js:415:16)
    at process._tickCallback (internal/process/next_tick.js:63:19)
    at Function.Module.runMain (internal/modules/cjs/loader.js:832:11)
    at startup (internal/bootstrap/node.js:283:19)
    at bootstrapNodeJSCore (internal/bootstrap/node.js:622:3)
  errno: 'ENOENT',
  code: 'ENOENT',
  syscall: 'spawn C:\\Windows\\system32\\cmd.exe',
  path: 'C:\\Windows\\system32\\cmd.exe',
  spawnargs: [ '/d', '/s', '/c', '"npm run lint"' ],
  cmd: 'npm run lint' }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants