Review: Approve

This looks good to me, approving.

Extremely minor nits that absolutely do not block merging would be:

- use of magic numbers; it'd be nice not to have to remember that 4 == friday; 
not sure if the python libraries have a constant defined for this.

- a lot of our scripts do a lot o stuff inline, resuliting in long code 
sequences that an be overwhelming. This makes things slightly harder to 
understand and harder to write unit tests for. I don't necessarily think this 
added check is worth bothering with a separate function (expecially because 
most of the added bits is handling what to do if the check trips), but it's 
worth thinking about as we add things.

- if you're going to rebase or add commits after you've gone ahead and made a 
merge request, it's nice to add a reference link to the merge request in your 
commit message.

Thanks for the nice improvement!
-- 
https://code.launchpad.net/~alexmurray/ubuntu-qa-tools/+git/ubuntu-qa-tools/+merge/427704
Your team Ubuntu Bug Control is subscribed to branch ubuntu-qa-tools:master.


_______________________________________________
Mailing list: https://launchpad.net/~ubuntu-bugcontrol
Post to     : ubuntu-bugcontrol@lists.launchpad.net
Unsubscribe : https://launchpad.net/~ubuntu-bugcontrol
More help   : https://help.launchpad.net/ListHelp

Reply via email to