launchpad-reviewers team mailing list archive

[Aaron Bentley <aaron@xxxxxxxxxxxxx>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 12-04-26 02:33 PM, Richard Harding wrote:
> Review: Approve code*
> 
> Thanks Aaron, couple questions, but looks good and ok'ing:
> 
> - I'm curious why you didn't make the make_question_job into a
> factory method and use it like the other factory createXXXX
> methods?

I wanted to use it with multiple classes.  If it had been a method, I
wouldn't have been able to do that.

> - I know it's just in testing, but with
> make_user_subject_body_headers it'd be great to have either a
> namedtuple or dictionary coming back. Keeping the right params in
> the right order is more likely to break.

I've never encountered a namedtuple in Launchpad code, nor seen a
dictionary used that way.  I don't we should disparage returning
multiple values; it's a key Python feature.

Also, I just moved that code around-- it already returned multiple values.

> - In the final test can you add a check for some bit of the email
> to make sure it's the one you're expecting.

There are thorough tests to ensure that the output email is correct.
The purpose of this test is to check the integration with Celery.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk+ZqKYACgkQ0F+nu1YWqI1dPQCfaYoYCpfTKgoewKdqvFdBR7c4
+NsAoIUuxjBhTttgPEyqRq3sJ+Wigi9n
=JztV
-----END PGP SIGNATURE-----

-- 
https://code.launchpad.net/~abentley/launchpad/celery-everywhere-9/+merge/103723
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.