search

Turbo87 / webpack-notifier

webpack + node-notifier = build status system notifications
ISC License
306 stars 41 forks source link

Add option to skip notifications on first build. #21

Closed etlovett closed 7 years ago

etlovett commented 7 years ago

With complex build configs, you can end up with lots of notifications on the initial build, which can be annoying, so it's convenient to have the option to skip them on the initial build and only get them on incremental builds (when using alwaysNotify: true). It was sufficiently trivial to add it that I decided to open a full PR rather than ask about it via an issue first.

I've tried to match the style of the existing code, but let me know if there's something you'd like tweaked. I'm also open to bikeshedding on the option name, if you have something else you'd prefer.

Turbo87 commented 7 years ago

@etlovett I'm wondering if that should actually be the default behavior...

etlovett commented 7 years ago

@Turbo87 In my opinion that comes down to whether most projects that use this have simple or complex build configurations. I like being notified on initial build for a simple configuration that produces one or maybe two notifications, but the flurry of notifications gets annoying with more complex configurations that build 5+ packages. I would assume that most users (particularly those that prefer the out-of-the-box experience) have relatively simple build configurations, and so therefore leaving notifications on by default is better.

That said, I'm happy to make the switch if you like.

Turbo87 commented 7 years ago

problem is that I've stopped using webpack quite some time ago so it's hard for me to judge what's best anymore...

etlovett commented 7 years ago

Alright, so how would you like to proceed? I'd choose landing this as-is because it aligns with my (data-limited) assumptions about Webpack usage and doesn't require reworking things. I'm happy to change it though if you'd prefer to do so before merging.

Turbo87 commented 7 years ago

@etlovett done, thanks!

etlovett commented 7 years ago

@Turbo87 Great, thanks! What's the path to getting this change published in npm, and is there anything I can do to help? I'd love to be able to use this flag in my project.

Turbo87 commented 7 years ago

@etlovett should be published as v1.5.0 now

etlovett commented 7 years ago

@Turbo87 Excellent, thanks!