Code review comment for lp:~danilo/launchpad/bug-820511

Revision history for this message
j.c.sackett (jcsackett) wrote :

This looks good. It'll certainly be easier to debug jobs this way.

One question, though.

> 18 @@ -262,7 +265,8 @@
> 19 def _logOopsId(self, oops_id):
> 20 """Report oopses by id to the log."""
> 21 if self.logger is not None:
> 22 - self.logger.info('Job resulted in OOPS: %s' % oops_id)
> 23 + self.logger.info('Job resulted in OOPS: %s' % (
> 24 + oops_id))
> 25 self.oops_ids.append(oops_id)

I'm not sure I see the need for this change; it doesn't look like the line was over the character limit, and functionally the two are identical, aren't they?

Given that I think they're identical, this doesn't block approval, I'm just curious what the intent was.

review: Approve

« Back to merge proposal