Skip to content

Conversation

@dmanchon
Copy link

No description provided.

@technige
Copy link
Contributor

That was quick, thank you :-)

My thoughts on the code:

  1. We should probably wrap this up into a small helper function to reduce duplication: get_host_port(socket) -> host, port.
  2. Explicit checks on AF_INET, rather than a simple else would ensure this can't happen again. If the address family isn't recognised at all, we should raise a ProtocolError.
  3. Tests.

Have you signed our CLA? I'm happy to work on this otherwise.

@dmanchon
Copy link
Author

Agree with all. I still didnt figure out yet how correctly run the tests.

Thanks.

@technige
Copy link
Contributor

./runtests.sh should do it though you'll need to pip install boltkit -r test/requirements.txt first.

@dmanchon
Copy link
Author

Thanks. I just agreed the CLA and i the tests passed in my local machine but unfortunately they are failing in the CI

@technige
Copy link
Contributor

Closing this PR as this has now been added in 6c5f6fb

@technige technige closed this Feb 16, 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.

2 participants