Code review comment for lp:~vila/udd/analyze-log-imports

Revision history for this message
John A Meinel (jameinel) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 09/05/2011 10:08 AM, Vincent Ladeuil wrote:
> Vincent Ladeuil has proposed merging lp:~vila/udd/analyze-log-imports into lp:udd with lp:~vila/udd/analyze_log as a prerequisite.
>
> Requested reviews:
> Ubuntu Distributed Development Developers (udd)
>
> For more details, see:
> https://code.launchpad.net/~vila/udd/analyze-log-imports/+merge/74057
>
> Based on https://code.launchpad.net/~vila/udd/analyze_log/+merge/74056, this patch parses and reports the successful imports found in the log file.
>
> This diverges a slight bit from the initial purpose of the
> analyze_log.py purpose. If more divergences appear, it may be
> nice to switch to a more general approach
> s/report_times(/report('times', or report('imports', / and allow
> some command-line or config parameter to decide which parts
> should reported but I don't it need for now.
>
>

I'm not sure I understand what you mean by "diverges from the initial
purpose". Could you clarify a bit?

Would you want to add some test case to the matching logic? (They should
be fairly easy to write as unit tests.)

+ # Cough, not really times... cough

^- what are they then? The comment doesn't really add any clarity.

 review: needsinfo

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk5l1f4ACgkQJdeBCYSNAAOyFwCfUAnmHFltJWBpDzhRoUXaMflc
yUcAoLS9QEy3P7+OskmNR5j0wILXFNMq
=mUIq
-----END PGP SIGNATURE-----

review: Needs Information

« Back to merge proposal