Code review comment for lp:~james-w/launchpad/register-code-import

Revision history for this message
Aaron Bentley (abentley) wrote :

It's nice of you to defer requesting a review until all tests are passing, but I think a lot of us don't observe that.

Was there a preimplementation call?

Was there any lint?

AIUI PEP8 requires that items within a class definition be separated by a single blank line, so a bunch of your webservice_error(400) lines need a blank line in front of them.

Why are you using export_write_operation rather than export_factory_operation on branchtarget?

On IHasCodeImports, you claim that newCodeImport returns BranchMergeProposal. I think this is inaccurate.

What would you think about expressing xx-code-import.txt's "Exceptions" heading as a unit test? Failing that, it needs two blank lines separating it from the previous section.

review: Needs Information

« Back to merge proposal