Code review comment for lp:~allenap/maas/more-better-things--bug-1389007
Revision history for this message
| Gavin Panella (allenap) wrote | # |
© 2004
Canonical Ltd.
•
Terms of use
•
Data privacy
•
Contact Launchpad Support
•
Blog
•
Careers
•
System status
•
67d34a1
(Get the code!)
> It'd be worth noting in the documentation (or did I miss it?) that
> this is a modified version of the Twisted implementation, and that it
> does not change the on-the-wire protocol
Done.
>
> .
>
> I'd stick the module-level verifyClass invocations in tests.
Done.
>
> .
>
> I wasn't sure about AMPTestCase, but the AMPTest class almost
> certainly should say AMP32 instead of plain AMP.
Done.
>
> .
>
> For the bulk of the code, I can only assume that it's as good as the
> code we've been relying on anyway. So I'm voting Approve for the code
> as such — though I'll leave it to others (including Jenkins) to figure
> out how well the landing would fit into the grand plan.
Yeah, it's the same code, modified to our style conventions. The tests
have been modified to use testtools instead of Trial, but otherwise
they're the same.
Thanks again!