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

Automagically Bind notify Methods #306

Merged
merged 2 commits into from Feb 18, 2020
Merged

Automagically Bind notify Methods #306

merged 2 commits into from Feb 18, 2020

Conversation

danii
Copy link
Contributor

@danii danii commented Dec 27, 2019

Automagically bind notify methods on all wrapper classes so the notify method can be destructured within the import statement and not run into any this related errors.

Automagically bind `notify` methods on all wrapper classes so the notify method can be destructured within the import statement and not run into any `this` related errors.
@danii danii requested a review from mikaelbr February 14, 2020 20:01
Copy link
Owner

@mikaelbr mikaelbr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh! Missed this one! Thanks for submitting it. I get this can be frustrating when doing destructuring for instance. I think it looks fine. Could you remove package-lock.json from the commit also?

Revert the changes made to `package-lock.json` in the commit 789c544.
@mikaelbr
Copy link
Owner

Thanks!

@mikaelbr mikaelbr merged commit be3642b into mikaelbr:master Feb 18, 2020
@yvele
Copy link

yvele commented Apr 8, 2020

How is this breaking? 🤔

@mikaelbr
Copy link
Owner

mikaelbr commented Apr 8, 2020

Strictly not very breaking. The most notable breaking I found was sanitizing timeout on snoretoast handling (see changelog https://github.com/mikaelbr/node-notifier/blob/master/CHANGELOG.md#breaking-changes). But I also marked this as breaking in the changelog as it, in theory, does break from existing API.

@yvele
Copy link

yvele commented Apr 8, 2020

Thanks

This was referenced Mar 7, 2021
This was referenced Mar 15, 2021
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