Skip to content

Readability improvement of Connection::connect#85

Merged
repejota merged 1 commit intorepejota:developfrom
anthonysterling:fix-issue-57
Apr 5, 2017
Merged

Readability improvement of Connection::connect#85
repejota merged 1 commit intorepejota:developfrom
anthonysterling:fix-issue-57

Conversation

@anthonysterling
Copy link
Copy Markdown
Contributor

  • Added a private isErrorResponse method to check if response is of -ERR
  • Created some Exception factory methods to centralise/standardise Exceptions and their messages

Fixes #57

- Added a private isErrorResponse method to check if response is of -ERR
- Created some Exception factory methods to centralise/standardise Exceptions and their messages

Fixes #57
Comment thread src/Nats/Connection.php

$this->ping();
$ping_response = $this->receive();
if ($ping_response !== "PONG") {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I removed this as we could just check for -ERR and get the expected behaviour.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.06%) to 88.144% when pulling 5b4a973 on anthonysterling:fix-issue-57 into d1fc1da on repejota:develop.

1 similar comment
@coveralls
Copy link
Copy Markdown

coveralls commented Sep 4, 2016

Coverage Status

Coverage decreased (-0.06%) to 88.144% when pulling 5b4a973 on anthonysterling:fix-issue-57 into d1fc1da on repejota:develop.

@repejota repejota self-assigned this Sep 15, 2016
@dfeyer
Copy link
Copy Markdown
Contributor

dfeyer commented Mar 1, 2017

By reading the code look OK, @repejota what do you think ?

I can test this PR tomorrow more in details

@repejota
Copy link
Copy Markdown
Owner

repejota commented Apr 5, 2017

Thanks!!!!

@repejota repejota closed this Apr 5, 2017
@repejota repejota reopened this Apr 5, 2017
@repejota repejota merged commit cba653f into repejota:develop Apr 5, 2017
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.

4 participants