RoccoC / webpack-build-notifier

A Webpack plugin that generates OS notifications for build steps using node-notifier.
MIT License
162 stars 24 forks source link

fix: ignoreWarnings not working on webpack 5 #91

Closed ImLunaHey closed 3 months ago

ImLunaHey commented 11 months ago

if this PR isn't wanted im happy to maintain my own fork. 🙏

also im not 100% how to fix the error the tests are throwing. 🤔 if someone can point me in the right direction ill be happy to fix them up.

closes: https://github.com/RoccoC/webpack-build-notifier/issues/90 and https://github.com/RoccoC/webpack-build-notifier/issues/81

ImLunaHey commented 10 months ago

Let me get this fixed up. 🙏

ImLunaHey commented 10 months ago

@RoccoC let me know if that resolves all the issues. 🙏

ImLunaHey commented 10 months ago

bump ⬆️

RoccoC commented 10 months ago

bump ⬆️

There's an issue when running npm test, but otherwise all looks good! Thanks again, and so sorry for the delay!

ImLunaHey commented 3 months ago

if tests need to pass before merging then a CI action should be added to enforce that.

ImLunaHey commented 3 months ago

i cannot get the tests to run locally. they just sit there without doing anything.

➜  webpack-build-notifier git:(master) ✗ npm run test -- --verbose

> webpack-build-notifier@3.0.0 test
> jest --verbose

 RUNS  tests/test.spec.ts
RoccoC commented 3 months ago

i cannot get the tests to run locally. they just sit there without doing anything.

I'll get these fixed up and merged. Thanks for your PR.

RoccoC commented 3 months ago

if tests need to pass before merging then a CI action should be added to enforce that.

Thanks for flagging this. Looks like TravisCI was no longer working. I went ahead and migrated to CircleCI.

ImLunaHey commented 3 months ago

@RoccoC kinda an odd move to repush this as your own commit and wipe me from the authors...

Screenshot 2024-05-30 at 8 59 03 AM
RoccoC commented 3 months ago

@RoccoC kinda an odd move to repush this as your own commit and wipe me from the authors...

Ah crap, @ImLunaHey , this was completely unintentional. I had first merged your PR into a new branch so I could fix the test issues, but when I merged that branch into master I didn't consider that all the work would be misattributed to me. 🫤

Interestingly, the auto-generated release notes captured your contribution.

I apologize for messing this up. Let me see if I can amend the commit history.

chadlwilson commented 3 months ago

Seems a bit unusual, generally if one rebases and rewrites history the "author" is retained, just committer changes. Think you'd probably need to rewrite history and force push on master to fix that for @ImLunaHey :-(

RoccoC commented 3 months ago

Think you'd probably need to rewrite history and force push on master to fix that

Exactly, thanks! 😄

Screenshot 2024-05-29 at 7 29 33 PM

@ImLunaHey , thanks again for the contribution, and apologies for my screw-up here

chadlwilson commented 3 months ago

(Sorry I ended up here because I posted a stupid question about whether this broke Webpack 4 compat, but saw up above that it does so deleted my comment - also see the release notes were clarrified that Webpack 4 support/compat was dropped so thank you for that!)

RoccoC commented 3 months ago

Ha, no worries! I was looking for that comment so I could reply 🙂

chadlwilson commented 3 months ago

Also thank you for putting in the effort to correct the contrib history here. It can seem like a small thing, but personally I feel these such small things are really important for the vibe/culture of OSS, especially now LLMs are laundering all our contributions everywhere else. It's a thankless task at the best of times! 😬

ImLunaHey commented 3 months ago
ImLunaHey commented 3 months ago

gg git 😅