[tor-dev] Stem code review 2012-12-21

Sean Robinson seankrobinson at gmail.com
Sat Dec 22 15:41:02 UTC 2012


Stem devs,

This is a review of Stem commits from 2012-12-11 through 2012-12-21.

Damian, you are very good at documentation, concise and informative.  I
tend to hit one or the other, but not both.  I recognize when someone gets
the right mix.  Good work.

I, too, have found logging.basicConfig[0] hides too much about how it works
and, now, I roll my own logging handler early.

Would GETINFO/GETCONF cache hit logging[1] benefit from log_once?  Or even
something between log and log_once (e.g. rate_limited_log)?

Is there any particular reason for moving away from readthedocs.org?  I'm
curious if there was more than a desire to self-host as much as possible.

Good work on the Controller.get_conf() clean-up[2].  This is easier to code
against with the predictable return types.  I have already written a new
method with this and always getting a list (event empty) reduces the amount
of error checking I had to do.

The heartbeat time tracking is a good idea[3].  But, I wonder, should this
bother with an accessor method?  Could this work as just well as an
(non-callable) attribute?

[0]:
https://gitweb.torproject.org/stem.git/commit/a7b275a3d64301da4d23137dbadd4842cc7a041f
[1]:
https://gitweb.torproject.org/stem.git/commit/9b5ff19be35ebbf8c75772a63406ba23762b37de
[2]:
https://gitweb.torproject.org/stem.git/commit/5b45d3125c811067d7c3ba0a11e324423c88d925
[3]:
https://gitweb.torproject.org/stem.git/commit/6bc92816dc4356a204fdde0f160ee344875143a3

-- 
Sean Robinson
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.torproject.org/pipermail/tor-dev/attachments/20121222/701d5699/attachment.html>


More information about the tor-dev mailing list