Setup hooks in package.json #36
Conversation
install.js
Outdated
|
|
||
| /** | ||
| * Get root dependent application path. | ||
| */ |
There was a problem hiding this comment.
Please use single line comment //
|
This is a great idea, I love it. However, it looks like the tests are failing, can you look into it? Also, would you mind updating the README describing how to use guppy with these changes? Also, we should probably add some functionality to guppy-cli for adding a hook(s) to the "guppy-hooks" config. If you feel up to tackling that as well, that would be very helpful. Thanks. |
|
If you rebase your branch on the latest from master, that should fix your build. |
install.js
Outdated
| var pack = require(rootApplicationPath() + '/package.json'); | ||
|
|
||
| // Install each confgured hook. | ||
| pack['guppy-hooks'] && async.each(pack['guppy-hooks'], function (hook, next) { |
There was a problem hiding this comment.
I would prefer it if you use an if here instead. That would make it more clear what's happening on this line and easier to maintain.
There was a problem hiding this comment.
Consider it done. Do you have any applicable code-style which I should follow for further contributions?
|
I'm glad you enjoyed the idea. I'll try and rebase from master to fix the build. Question: are the guppy-hook packages still applicable after this new method of install? Should I replace the README documentation on that part, or should I just add an alternative method of install for the hooks? |
lucasconstantino
force-pushed
the
install
branch
from
95c55aa
to
77019fa
Compare
lucasconstantino
force-pushed
the
install
branch
from
77019fa
to
c05f010
Compare
|
@therealklanni I've modified code as requested. I've also created a pull-request to my (pull-requesting) branch so to make it easier for us to discuss about the README changes, but without blocking this pull-request from merging. I've made quite a huge rewriting to the README on that branch. Please take the changes mostly as suggestions. |
|
@lucasconstantino yes guppy-hook packages are still relevant, but they have more specific use-cases now. I'm going to pull this branch and do some testing locally as soon as I get a chance before merging. |
This pull-request aims to make it easier - and less bloating to the dependency system - to define hooks to be installed via package.json config. I've introduced a property named guppy-hooks, which should be fulfilled with an array of hooks to install.
I've also removed guppy-pre-commit dependency, so to use this new method of install.