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

> 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?

No, but I chatted with wgrant about the strategy.

> Was there any lint?

I don't know, running "make lint" runs it on lots of unexpected files.

> 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.

Done.

> Why are you using export_write_operation rather than export_factory_operation
> on branchtarget?

That's not supposed to be there any more, deleted.

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

Fixed.

> 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.

I don't know how to do webservice tests as unit tests, if you can point me to
examples I will convert them.

Blank lines issue fixed anyway, and I updated the text to make the intent of
the tests clearer as well.

Thanks,

James