Conform to PEP8 - Githubissues

super3 commented 8 years ago

Also rename vars shadowing built-ins.

yimingliu commented 8 years ago

Thanks! I haven't had a chance to test it, but I've been meaning to PEP8-fy this module and just never had the time.

Could I get you to do two minor things?

In the new NatPMP class, the proto variable is hard-coded inline to 1 and 2. Could use the NATPMP_PROTOCOL_TCP and NATPMP_PROTOCOL_UDP constants already in the module to make it more readable.
I wish something netifaces existed back when I wrote this first, and would definitely have used it, since gateway detection was the most fragile part of the code. However, now I have legacy code deployed based on NATPMP.py, that doesn't rely on installing a third-party dependency. Can you do an ImportError exception check in get_gateway_addr to see netifaces can be imported, and fallback to the netstat-based hack otherwise?

super3 commented 8 years ago

No problem. Part of our own PEP8 cleanup, so I figured best to contribute back.

I'll tackle them tomorrow morning. Tested locally and on Travis-CI. Should be zero functional changes. On Jan 19, 2016 6:35 PM, "yimingliu" notifications@github.com wrote:

Thanks! I haven't had a chance to test it, but I've been meaning to PEP8-fy this module and just never had the time.

Could I get you to do two minor things?

-

In the new NatPMP class, the proto variable is hard-coded inline to 1 and 2. Could use the NATPMP_PROTOCOL_TCP and NATPMP_PROTOCOL_UDP constants already in the module to make it more readable.

I wish something netifaces existed back when I wrote this first, and would definitely have used it, since gateway detection was the most fragile part of the code. However, now I have legacy code deployed based on NATPMP.py, that doesn't rely on installing a third-party dependency. Can you do an ImportError exception check in get_gateway_addr to see netifaces can be imported, and fallback to the netstat-based hack otherwise?

— Reply to this email directly or view it on GitHub https://github.com/yimingliu/py-natpmp/pull/8#issuecomment-173023410.

super3 commented 8 years ago

@yimingliu Believe I implemented what was requested.

super3 commented 8 years ago

Note credit goes to @robertsdotpm for the get_gateway_addr() with netifaces. I just did the PEP8 stuff.

yimingliu / py-natpmp

Conform to PEP8 #8

In the new NatPMP class, the proto variable is hard-coded inline to 1 and 2. Could use the NATPMP_PROTOCOL_TCP and NATPMP_PROTOCOL_UDP constants already in the module to make it more readable.