Code review comment for lp:~lifeless/launchpad/merge

Revision history for this message
Robert Collins (lifeless) wrote :

Ugh, there are tabs in teh diff. Fail-vim-rc-in-vm. will fix.

There are already tests for pasing in a revid to setStatus(QUEUED, revision_id=).

mergeFailed seems damaged, it uses self.merger, which isn't defined or used anywhere.

If I can change its definition, I can call it - so I'll do that change.

...

Have looked into it, mergeFailed isn't used anywhere, isn't exposed in API's and is buggy today (it doesn't dequeue completely). Its behaviour isn't tested either. To fix it and make it safe to call any point will either make setStatus significantly more complex (have to call dequeue when moving from QUEUED in every case except when moving to MERGE_FAILED), or it will make mergeFailed complex (have to accept being called when not in state QUEUED).

I looked for and could not find docs saying about whether setStatus was being deleted, or the other functions were, or something else, and couldn't find anything, so I took the approach that leaves the least code, with no reduction in capabilities - deleting mergeFailed.

« Back to merge proposal