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

Add Message.Respond implementing #281 #283

Merged
merged 3 commits into from
Aug 27, 2019

Conversation

watfordgnf
Copy link
Collaborator

Implements #281 by adding Respond to Msg to simplify responses to Request-Reply.

I brought over the same changes to Subscription that landed in https://github.com/nats-io/nats.go/pull/489/files however this breaks TestConnection.TestClosedConnections which had certain assumptions about the exception messages.

Should we alter the unit test to keep parity with the go client, or should we alter the code to keep raising the same exception?

- Adds Respond to Msg to simplify responses to Request-Reply
- No longer clears Subscription.conn, until disposing
@ColinSullivan1
Copy link
Member

As much as I want to keep parity, imo we shouldn't change the exception being thrown. While unlikely users would run into that, it would break backward compatibility. We should keep the current exception...

@ColinSullivan1
Copy link
Member

Thanks! LGTM.

@ColinSullivan1 ColinSullivan1 merged commit d2cfbe6 into nats-io:master Aug 27, 2019
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.

2 participants