Code review comment for lp:~james-w/launchpad/copy-archive-job

Revision history for this message
Guilherme Salgado (salgado) wrote :

I haven't done a through review as I'm not familiar with the job system, so wouldn't be comfortable doing it. I have a few comments, though.

I assume the new tables created to describe a new job are standardised, but we need to have the new one reviewed by either stub or jml.

It's weird to see new code raising SQLObjectNotFound. Does the job system expect that?

I think your tests can do with just the DatabaseFunctionalLayer, which might be faster than LPZopelessLayer.

I'll be happy to do a thorough review tomorrow, if you think it's worth it, but I'd like someone more familiar with the job system to review it as well.

« Back to merge proposal