Code review comment for lp:~cjohnston/ubuntu-ci-services-itself/cli-stuff

Revision history for this message
Andy Doan (doanac) wrote :

20 --- cli/cli/changes_processor.py 2014-01-08 13:27:56 +0000
21 +++ cli/ci_libs/file_handler.py 2014-01-10 15:39:09 +0000
22 @@ -1,5 +1,6 @@
23 +#!/usr/bin/env python

Not a big deal - but since this is not a main entry point, you don't need line 23.

I see the basics of an http-get in status.ticket_status and there are _post/_patch in ticket.py. Not sure the where the code is going long-term, but it might be worth moving those to your utils module and have utils.[get|post|patch].

You assert the auth_config from file_handler.py but the config from utils.py. Maybe just do that in a single place?

cli/tests/test_get_ticket_status.py: you should add a test for a ticket that doesn't exist. I think your code is currently just going to throw an exception. So fixing the code and adding a test could ensure it handles the issue gracefully.

« Back to merge proposal