Code review comment for lp:~ricardokirkner/locolander/celery-tasks

Revision history for this message
Natalia Bidart (nataliabidart) wrote :

Looks good!

One note: you should not import os.path, but import os. From the help(os.path):

"Instead of importing this module directly, import os and refer to this module as os.path."

Any reason to change the prefix on l.94-95?

The import from l.158 should have a blank line separating import from stdlib from imports from third-party.

When needing the same patch in more than one test, and specially when the same patch is needed in every test inside the test case, I advice to do the patching in the setUp method, so we can have the patch in a single place and don't change the signature of every method (but if you disagree, you can leave as is.

Approving since none of the above is a blocker.

review: Approve

« Back to merge proposal