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

Re: [tor-bugs] #9376 [Pluggable transport]: subprocess management module for pyptlib



#9376: subprocess management module for pyptlib
---------------------------------+------------------------------------------
 Reporter:  infinity0            |          Owner:  asn           
     Type:  enhancement          |         Status:  needs_revision
 Priority:  normal               |      Milestone:                
Component:  Pluggable transport  |        Version:                
 Keywords:  pyptlib              |         Parent:                
   Points:                       |   Actualpoints:                
---------------------------------+------------------------------------------

Comment(by asn):

 Looks good. Two things:

 - Nice tests! Any chance they could get some small documentation? For
 example, what is `test_killal_kill` supposed to do, and how does it do it?
 It doesn't send any signals or anything.

 - You didn't document the `_Popen_defaults =
 zip(a.args[-len(a.defaults):],a.defaults); del a`. Can you document it?

 Some further comments. If you don't have time to do these, I can fix them
 myself before merging:

 - Hm. It seems like you moved all the `util.py` file to
 `util/__init__.py`. I don't like code in `__init__.py` except if there is
 a reason. Is there a reason for it?

 - I'm not really used to the following idiom (wrt proc_is_alive()):
 {{{
 if mswindows:
     def something():
         XXX
 else:
     def something():
         YYY
 }}}
 I prefer the following:
 {{{
 def something_impl_windows():
        XXX

 def something_impl():
        YYY

 if mswindows:
      something = something_impl_windows
 else:
      soemthing = something_impl
 }}}

-- 
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/9376#comment:9>
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