Code review comment for lp://staging/~diogobaeder/txstatsd/fix-host-lookup

Revision history for this message
Sidnei da Silva (sidnei) wrote :

A few comments:

1. I dislike class attributes with parameters that cannot be set through the constructor. How are you suppose to change LIMIT? By subclassing? By monkey patching?

2. Along the same lines, because you can't easily change LIMIT, the tests iterate calling write() up to 'LIMIT' times, which is useless. If LIMIT was configurable, you could set it to '1' in the tests and make them more efficient.

review: Needs Fixing

« Back to merge proposal