ubuntu-bugcontrol team mailing list archive
-
ubuntu-bugcontrol team
-
Mailing list archive
-
Message #04610
Re: [Merge] ~alexmurray/ubuntu-qa-tools:unembargo-warn-security-updates-on-fridays into ubuntu-qa-tools:master
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.
References