[tor-dev] Stem code review 2013-01-03

Damian Johnson atagar at torproject.org
Sun Jan 6 00:26:51 UTC 2013


Hi Sean. As always, thanks for the code review!

> I did find the 'assert extended == "EXTENDED"' test after is_ok() to be strange...

Oops, thanks for catching that. I don't mind a bit of paranoia in
making sure that the response is giving us a circuit id but I've
avoided using the assert keyword in our codebase. Changed...

https://gitweb.torproject.org/stem.git/commitdiff/9cd9b4004995015c0a0a04a32bb9c68a1addf728

> 1) There is a discrepancy in class naming (i.e. MicroExitPolicyRule vs MicrodescriptorExitPolicy) about which I vote to rename MicrodescriptorExitPolicy to MicroExitPolicy.

The reason for the naming difference is that
"MicrodescriptorExitPolicyRule" tripped an arbitrary threshold in my
head for 'this class name is way too long'. I agree,
"MicrodescriptorExitPolicy" is too long too. Changed...

https://gitweb.torproject.org/stem.git/commitdiff/ec306804fc325b8670758df36ec76a1e10fe3604

> But, considering the size results in 2)d), is it worth the time to optimize this area?

I'm starting to suspect not. The only-parse-if-we-need-to change has
helped but I'm not so sure about MicroExitPolicyRule. This was an
attempt to address 'https://trac.torproject.org/7831' but it's still
exhibiting issues. I'd be ok with reverting it if you want.

Cheers! -Damian


More information about the tor-dev mailing list