-
Notifications
You must be signed in to change notification settings - Fork 65
feat(unsubscribe): merge n1-unsubscribe plugin #49
Conversation
Integrate n1-unsubscribe into nylas-mail directly See: https://github.com/colinking/n1-unsubscribe
and fix out-of-date “N1” references with “Nylas Mail”
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'm 👍 on this, just curious about the listener change.
@@ -153,7 +153,7 @@ module.exports = | |||
setupEmitter: -> | |||
return if @_emitter | |||
@_emitter ?= new EventEmitter() | |||
@_emitter.setMaxListeners(50) | |||
@_emitter.setMaxListeners(100) |
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.
Just curios why the listener limit needed to be raised?
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.
@dweremeichik I had to raise the maximum to allow more for the unsubscribe plugin. There doesn't appear to be any memory leaks and is just the nature of how the plugin was implemented
@KyleKing could you make an issue on this repo for this? I know you already have two (that you have most graciously linked), but due to preservation sake I'd prefer the redundancy. Thanks! On another note, as a built in plugin, this should not change any behavior for deleting or archiving emails correct? This should be supplemental buttons/keymaps that allow this enhanced feature. |
@seesemichaelj the code appears to be scoped to the "new" internal unsubscribe package. |
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.
after the question from @dweremeichik is answered and he's good with this, this has my approval
Wow, Nylas Mail is still alive! Thanks for the quick responses. The plugin will call the Nylas API for archiving or deleting, so there is no impact on any of the default Nylas behavior @colinking and I are both welcome to input, so let us know what can be improved |
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.
@KyleKing the community is definitely trying to keep Nylas mail alive. Awesome work and initiative on bringing this plugin into the main project! I'm good with merging this in. Please feel free to join us in slack or other issues in the repo!
Moves the unsubscribe plugin into an internal package. * init: `unsubscribe` as internal package Integrate n1-unsubscribe into nylas-mail directly See: https://github.com/colinking/n1-unsubscribe * fix broken N1 api request and fix out-of-date “N1” references with “Nylas Mail”
Moves the unsubscribe plugin into an internal package. * init: `unsubscribe` as internal package Integrate n1-unsubscribe into nylas-mail directly See: https://github.com/colinking/n1-unsubscribe * fix broken N1 api request and fix out-of-date “N1” references with “Nylas Mail”
…l-lives#49) Moves the unsubscribe plugin into an internal package. * init: `unsubscribe` as internal package Integrate n1-unsubscribe into nylas-mail directly See: https://github.com/colinking/n1-unsubscribe * fix broken N1 api request and fix out-of-date “N1” references with “Nylas Mail”
This PR will merge the n1-unsubscribe plugin into an internal plugin within nylas-mail
The plugin both unsubscribes and archives emails with a single button press. Users have the option of setting the default action to trash emails instead of archiving, to set a keyboard shortcut, add an optional confirmation dialog, and there is decent multilingual support.
The PR to merge the unsubscribe plugin is well overdue and will close nylas/nylas-mail#3154, which happened right at the cusp of the N1 > Nylas Mail transition and the PR that occurred right at the official Nylas Team > OS Community transition: nylas/nylas-mail#3612