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

parser: rm unused OP_CONNECT & CONNECT_ARG. #55

Merged
merged 1 commit into from
Jan 20, 2016

Conversation

dwlf
Copy link
Contributor

@dwlf dwlf commented Mar 16, 2015

I was looking at the test coverage and this caught my eye.

@krobertson
Copy link
Contributor

Unsure about removing this. It is still on the server side, and while it is only referenced in the parser on the server side, it is essentially part of the protocol.

@dwlf
Copy link
Contributor Author

dwlf commented Mar 16, 2015

@krobertson ok, let me know as we can add the rest of the states as there is no where to go from nc.ps.state = OP_C . And add a test case.

@dwlf
Copy link
Contributor Author

dwlf commented Mar 16, 2015

I did notice that the node client using a different model does use CONNECT, https://github.com/derekcollison/node-nats/blob/master/lib/nats.js#L338 .

derekcollison added a commit that referenced this pull request Jan 20, 2016
parser: rm unused OP_CONNECT & CONNECT_ARG.
@derekcollison derekcollison merged commit 6c5b8c0 into nats-io:master Jan 20, 2016
@dwlf dwlf deleted the rm-unused-op-connect branch January 20, 2016 18:52
TheGhoul21 pushed a commit to TheGhoul21/nats.go that referenced this pull request Jun 20, 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.

None yet

3 participants