Comment 2 for bug 1917331

Hi Tom, my thoughts:

Turning the mp.nominateReviewer() Unauthorized exception into a non-fatal warning looks sensible to me. J-l-p should still be able to submit a "vote" (review) to the MP, which will then appear as a community review. There will be no indicator that a CI run is in progress in this case, as you noted, but the rest should just work. I can't think of anything else that could go wrong in the jlp CI workflow because of this change, but we should definitely test in on a live MP before merging it.

One concern I have: as you know jlp has two branches at the moment: the legacy py2 branch and master, which uses Python3. Most of the deployments use the py2 branch at the moment, but I thought it as a "frozen" branch with no further development. We can certainly still land changes to it, but in this case we should make the effort of testing and committing them on master too, otherwise we'll have bad surprises in the future.

The package uses the unittest framework. I can run the tests from the master branch with:

  python3 -m unittest

IIRC running the Python 2 tests should be the same, but using the python2 interpreter and the py2 branch.