Review: Approve
Jonathan--
This looks good. I have two quibbles about comments in the code below, but
that's all.
> === modified file 'lib/lp/services/tests/test_utils.py'
> --- lib/lp/services/tests/test_utils.py 2011-12-19 23:38:16 +0000
> +++ lib/lp/services/tests/test_utils.py 2012-05-31 12:14:21 +0000
> @@ -384,3 +388,14 @@
> """Values are obfuscated recursively."""
> obfuscated = obfuscate_structure({'foo': (['a...@example.com'],)})
> self.assertEqual({'foo': [['<email address hidden>']]}, obfuscated)
> +
> +
> +class TestTotalSeconds(TestCase):
> +
> + # Remove this when Python 2.6 support is dropped. Replace calls with
> + # timedelta.total_seconds.
> +
> + def test_total_seconds(self):
> + # Numbers are arbitrary.
> + duration = timedelta(days=3, seconds=45, microseconds=16)
> + self.assertEqual(3 * 24 * 3600 + 45.000016, total_seconds(duration))
Could you make the comment in this test an XXX comment? It marginally
increases the chances other people will notice this and kill it appropriately.
> === modified file 'lib/lp/services/utils.py'
> --- lib/lp/services/utils.py 2012-05-24 20:25:54 +0000
> +++ lib/lp/services/utils.py 2012-05-31 12:14:21 +0000
> @@ -26,6 +26,7 @@
> 'save_bz2_pickle',
> 'synchronize',
> 'text_delta',
> + 'total_seconds',
> 'traceback_info',
> 'utc_now',
> 'value_string',
> @@ -383,3 +384,15 @@
> for key, value in o.iteritems())
> else:
> return o
> +
> +
> +def total_seconds(duration):
> + """The number of total seconds in a timedelta.
> +
> + In Python 2.7, spell this as duration.total_seconds(). Only needed for
> + Python 2.6 or earlier.
> + """
> + return (
> + (duration.microseconds +
> + (duration.seconds + duration.days * 24 * 3600) * 1e6)
> + / 1e6)
This should probably have a similar XXX comment.
I believe I mentioned the LoC tool in IRC; as you mentioned, this is part of a
broader arc of work--if that's going to end up reducing LoC, fantastic. If not,
you still have that credit I mentioned, so either way this should be fine as
regards our LoC limits.
Thanks!
--
https://code.launchpad.net/~jml/launchpad/generate-ppa-logging/+merge/108040
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
