Fwd: Re: [icedtea-web] xml output for junit, transformation sheets for daily report

[Dr Andrew John Hughes]

On 14:15 Mon 09 May     , Omair Majid wrote:
> Hi Jiri,
> 
> Overall, I think the patch looks good. A few concerns:
> 
> >
> > 2011-05-09 Jiri Vanek <jvanek at redhat.com>
> >
> > * tests/junit-runner/JunitLikeXmlOutputListener: This listener exports
> > results of junit in xml which "follows junit-output schema". Extended
> > for date, duration and some statististics for future purpose
> > * tests/junit-runner/CommandLine.java: added usage of
> > JunitLikeXmlOutputListener added skipping of inner classes and one
> > jnlp file from sources package.
> > *index.html: file which provides runtime transformation
> > of tests-output.xml and report.xml
> 
> this file should be named tests/sytles/index.html.in
> 
> > * tests/styles/index.js: runtime transformation script and fast
> > navigation functions
> > * tests/styles/report.css: styles for transformed result
> > * tests/styles/jreport.xsl: template for human-readable xml->html
> > transformation.
> 
> Could you possibly rename the styles dir? Perhaps 'report' might be more 
> appropriate.
> 
> I also noticed that the patch contains lots of blank lines. Some of the 
> really short (1-line) methods contain many blank lines (2 or more). 
> Please delete them if you can.
> 
> Other than that, this looks fine to me. I would love to get a second 
> opinion, though.
> 
> Cheers,
> Omair

The code also needs documenting, though that could be done in a follow-up patch.
-- 
Andrew :)

Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

Support Free Java!
Contribute to GNU Classpath and IcedTea
http://www.gnu.org/software/classpath
http://icedtea.classpath.org
PGP Key: F5862A37 (https://keys.indymedia.org/)
Fingerprint = EA30 D855 D50F 90CD F54D  0698 0713 C3ED F586 2A37