Code review comment for lp:~gary/launchpad/bug821531

Revision history for this message
Benji York (benji) wrote :

This branch looks good. I can't wait to try it out. Here are some
relatively minor comments.

The name "pstats" might be a little better than "stdlib" because it's a
pstats-formatted file that's generated. For that matter, now that there
are more than one output formats, "log" might be better replaced with a
word that describes the generated format ("callgrind") while perhaps
leaving "log" as a backward-compatible alias.

Line 111 of the diff: the docstring summary for StdLibStats wraps.

Line 130 of the diff: JavaScript has warped your brain, you're missing a
comma on the last element of "mapping".

Line 183 of the diff, teeny-tiny suggestion: you could use '-'.join(...)
instead of '%s-%s-%s-%s' % (...).

Lines 235 and 256 of the diff: more docstring wrapping. Maybe those
should just be comments instead.

review: Approve (code)

« Back to merge proposal