Code review comment for ~ilasc/launchpad:add-mp-url-to-git-push

Revision history for this message
Ioana Lasc (ilasc) wrote :

Added more Unit Tests and addressed almost all comments apart from

from six.moves.urllib.parse import quote instead of just importing urllib

can't get the code to compile with it - which is a known PyCharm issue, tried running cli tests but code doesn't run in cli either and make lint fails.

I believe there is also an issue with Unauthorized scenario in getMergeProposalURL. I went by below comment and instead of raising Unauthorized I'm returning '', but now I'm not sure if that's not meant for timeouts only?

Comment I'm referring to in git.py line 564:
            # XXX cjwatson 2019-05-09: It would be simpler to just raise
            # this directly, but turnip won't handle it very gracefully at
            # the moment. It's possible to reach this by being very unlucky
            # about the timing of a push.

« Back to merge proposal