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

Fix issue with chainable on calls in pg. #177

Closed
wants to merge 1 commit into from

Conversation

sebastianhoitz
Copy link
Contributor

EventEmitters on method is chainable as documented here: http://nodejs.org/api/events.html#events_emitter_on_event_listener

By not returning the handler you introduce a breaking behavior that other libraries (like SequelizeJS) might rely on.

@txase
Copy link

txase commented Oct 14, 2014

@sebastianhoitz Good catch! Thanks for sending us the PR. We should be able to get this into our release this week.

@wraithan
Copy link
Contributor

@sebastianhoitz So we should be merging this in this week. I was wondering how you prefer to be attributed in the release notes. We are happy to use your name as it appears on github, your github handle, or any other identifier you prefer.

@wraithan
Copy link
Contributor

@sebastianhoitz Your patch was merged out of band (and rebased). Closing this PR. Thanks a ton for the patch!

@wraithan wraithan closed this Oct 16, 2014
jsumners-nr pushed a commit to jsumners-nr/node-newrelic that referenced this pull request Apr 16, 2024
bizob2828 pushed a commit to bizob2828/node-newrelic that referenced this pull request Jul 26, 2024
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.

3 participants