Code review comment for lp:~benji/launchpad/bug-580035

Revision history for this message
Māris Fogels (mars) wrote :

Hi Benji,

Once again, these changes look good. I really like the use of named variables for time durations in ensure_sane_signature_timestamp(). It really improves the readability.

One question I did have: did you have a pre-implementation call with the bugs team about this? IIRC at the last EPIC gmb was in the process of killing all doctests in bugs. I would think the bugs team members would have asked for unit tests of this new feature.

If it turns out that the bugs team does in fact want unit tests, then I am sure you can land this as-is and land a follow-up branch soon after.

Looks good, r=mars.

Maris

review: Approve

« Back to merge proposal