Kali

Merge lp:~jerith/kali/general-dev into lp:kali

  1. general-dev
  2. Merge into trunk
Proposed by Jeremy Thurgood
Status: Merged
Approved by: Tristan Seligmann
Approved revision: 127
Merged at revision: not available
Proposed branch: lp:~jerith/kali/general-dev
Merge into: lp:kali
To merge this branch: bzr merge lp:~jerith/kali/general-dev
Reviewer Review Type Date Requested Status
Tristan Seligmann Needs Fixing
Review via email: mp+2949@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Jeremy Thurgood (jerith) wrote :

Fixes two reported bugs, adds a couple of tests and fixes the login thing for non-opers that doesn't have a ticket..

lp:~jerith/kali/general-dev updated
126. By Jeremy Thurgood <email address hidden>

Added docstrings to tests.

Revision history for this message
Tristan Seligmann (mithrandi) wrote :

Yay, unit tests!

1. Perhaps getAuthenticatedAvatar should be named createAvatar?

2. Instead of defining __getattr__ on FakeProtocol, addStub should just create the function object and set it as an attribute on self. Also, I don't think "stub" is the right name for this, but whatever.

3. assertTrue is nicer than assert_.

4. Instead of assertEqual(type(foo), Quux), consider assertIsInstance(foo, Quux).

review: Needs Fixing
Revision history for this message
Jeremy Thurgood (jerith) wrote :

1. Will do.

2. I thought about that. As soon as we want to modify it to deal with multiple calls to the same function, creating the function instead of using __getattr__ stops working without a whole lot of extra plumbing.

3. It is.

4. I shall.

Revision history for this message
Tristan Seligmann (mithrandi) wrote :

Okay, I guess the __getattr__ can stay for now. Once you've made the other changes, I think this should be good to merge.

lp:~jerith/kali/general-dev updated
127. By Jeremy Thurgood

Changes suggested in review.

Subscribers

People subscribed via source and target branches