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.

Sign up for GitHub

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

Jump to bottom

Clear up drain listeners in the case of an error #149

Merged
merged 1 commit into from
Aug 17, 2017
Merged

Clear up drain listeners in the case of an error #149

merged 1 commit into from
Aug 17, 2017

Conversation

mcollina
Copy link
Collaborator

Fixes #148.

@mcollina mcollina requested review from behrad and GavinDmello August 16, 2017 15:16
@coveralls
Copy link

coveralls commented Aug 16, 2017 •
edited
Loading

Coverage Status

Coverage increased (+0.2%) to 97.507% when pulling effb43a on drain into f084a2b on master.

@coveralls
Copy link

coveralls commented Aug 16, 2017 •
edited
Loading

Coverage Status

Coverage increased (+0.2%) to 97.507% when pulling effb43a on drain into f084a2b on master.

@coveralls
Copy link

coveralls commented Aug 16, 2017 •
edited
Loading

Coverage Status

Coverage increased (+0.2%) to 97.507% when pulling d6032bd on drain into f084a2b on master.

GavinDmello
GavinDmello reviewed Aug 17, 2017
View reviewed changes
lib/client.js Outdated
@@ -265,6 +270,11 @@ Client.prototype.close = function (done) {
}
}

function callCb (cb) {
// needed to work around a problem in Node 4.x
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's was the problem in Node 4 ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There is no need to do all of that in fact. Updated.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 97.497% when pulling 7f1b56b on drain into f084a2b on master.

1 similar comment
@coveralls
Copy link

coveralls commented Aug 17, 2017 •
edited
Loading

Coverage Status

Coverage increased (+0.1%) to 97.497% when pulling 7f1b56b on drain into f084a2b on master.

@GavinDmello
Copy link
Collaborator

The coverage took a hit

@mcollina
Copy link
Collaborator Author

it took a hit because I removed some code from the previous commit, it's still +0.1% with master. I'll adjust the settings.

@GavinDmello GavinDmello requested review from GavinDmello and removed request for GavinDmello August 17, 2017 08:16
@GavinDmello
Copy link
Collaborator

These changes look okay to me 👍

@mcollina mcollina merged commit 10ab8e3 into master Aug 17, 2017
@mcollina mcollina deleted the drain branch August 17, 2017 08:20
@nadalizadeh
Copy link

Thanks for fixing this,
How can I upgrade to this latest commit using npm?

@GavinDmello
Copy link
Collaborator

It should be in there in the latest release. Just upgrade your aedes package

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GavinDmello GavinDmello GavinDmello left review comments

@behrad behrad Awaiting requested review from behrad

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mcollina @coveralls @GavinDmello @nadalizadeh