[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [tor-bugs] #9485 [Pluggable transport]: Cleanup of pyptlib internals

#9485: Cleanup of pyptlib internals
-------------------------------------+---------------------
     Reporter:  infinity0            |      Owner:  asn
         Type:  defect               |     Status:  new
     Priority:  normal               |  Milestone:
    Component:  Pluggable transport  |    Version:
   Resolution:                       |   Keywords:  pyptlib
Actual Points:                       |  Parent ID:
       Points:                       |
-------------------------------------+---------------------

Comment (by asn):

 Comments after first code review:

 * Why is `fromEnv()` a classmethod that returns an instance of the class?
 Why don't you do that stuff in the __init__ of the class? I'd prefer if it
 was so, if there is no real reason for `fromEnv` to be a classmethod.

 * `env_id` is unused.

 * Why do you call the function `getServedTransports`? Why ''Served''? Same
 with `getServedBindAddresses`. Served by whom and where? I think I would
 prefer `getTransports` or something.

 * Can you make it clearer that `getServedBindAddresses` returns a dict?
 Also, how are people supposed to get the address of the ExtendedORPort, or
 the state location with the new API? Can this be mentioned in the sphinx
 docs?

 * You didn't update the docs for the `reportMethodSuccess` API change.

 * In `EnvError`, is the following correct? Doesn't that return a boolean?
 {{{
     def __str__(self):
         return self.message or self.cause.message
 }}}

 * Please do the :raises: doc dance in exception-raising places like
 `CheckClientMode` and `env_has_k`.

 * Have you used pylint (or any other static analyzer) on the new code? Did
 it find anything fix worthy (unused vars, unused imports, etc.)

 Apart from the above, we need to write patches for obfsproxy and sshproxy,
 and then we are ready to merge :)

-- 
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/9485#comment:3>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs