-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Conversation
Current coverage is
|
|
||
beforeInstall: function (options) {}, | ||
afterInstall: function (options) {} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would remove the boilerplate in here and the path require
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
by path require, you mean line 21 - 23?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line 1 of blueprints/reducers/index.js. You can have it just export an empty object since you don't need to override any of the hooks.
I think there may already be a duck
blueprint that does something like this, but might be useful to have different versions of blueprints.
Generic but non-automatic response: away on vacation, ty for PR, will review next Saturday/Sunday. |
We already have a duck reducer (naming is a bit unclear for the uninitiated), so is this needed? |
@davezuko I didn't realize the duck was supposed to be a reducer blueprint. And it doesn't follow the other blueprint, but meh! do as you like! |
I'll consider it further, but will probably drop one or the other. Sent from my iPhone
|
This will be changing with #684. Thanks for the contribution though, much appreciated! |
The reducers are what I use the blueprints for the most so far, figured I would contribute back my blueprint for reducers.
If you don't want a reducer blueprint, feel free to reject.