On 10/01/2013, at 8:07, Adam Murdoch <[email protected]> wrote:
>
> On 10/01/2013, at 9:40 AM, Marcin Erdmann wrote:
>
>> Looks like I've finally managed to more less finish the first implementation
>> step ('Add a build dashboard report') of the reporting design document. It
>> might still be rough around the edges so I'm looking forward to your
>> feedback to be able to polish it a bit more.
>>
>> Code is available at:
>> https://github.com/erdi/gradle/tree/reporting-improvements
>
> The code looks really good.
>
>>
>> Following are the questions/concerns/deficiencies I have/see:
>>
>> - There is only a little bit of documentation in there, can you please tell
>> me what are the expectations in this area, preferably with some code
>> examples? Which parts definitely have to be javadoced? How do I document
>> DSLs?
>
> Generally, I would expect something like this:
>
> - Some kind of mention of the plugin in the user guide - perhaps a reporting
> chapter, or perhaps a plugin specific chapter. It doesn't need to be very
> detailed initially and can grow over time as we add more stuff. I wouldn't
> expect more than a couple of paragraphs initially.
> - A sample, referenced by the user guide.
> - Javadocs on the public types and methods (running `gradle docs:check` will
> complain if you've missed something).
> - The public types referenced in the dsl guide (have a look in
> subprojects/docs/src/docs/dsl).
>
>>
>> - Apart from tests everything is written in Java as I don't know where in
>> production code I'm allowed or even should use Groovy. Some code, the plugin
>> class in particular, would be much simpler if I was allowed to write it in
>> Groovy. I'm happy to rewrite anything you think should be coded in Groovy.
>
> It's fine to use groovy for the plugin and task implementations.
I'm generally opposed to using Groovy for tasks because Groovydoc sucks so
much. However, as Adam points out we don't have a hard rule for this.
I'd do as you say; just rewrite the plugin.
> I wouldn't use it for any of the other production code. We would always
> implement the unit and integration tests using spock (+ groovy, of course).
>
>>
>> - The dashboard is dead simple and might use some better styling, but I'm
>> not a designer so I would appreciate really precise directions in that
>> matter. It's currently using base-style.css and the header looks similar to
>> that in test results. Please also check if you are happy with the wording
>> used.
>>
>> - I believe test coverage is decent, but if you see anything missing please
>> let me know. Report generating POJO is unit tested and task + plugin are
>> integration tested.
>
> Tests look good to me.
>
>>
>> - I added displayName property to Report, currently the implementation for
>> task generated reports holds values like 'Report generated by task
>> ':subproject:codenarcMain' (html)', is that ok?
>>
>> - In BuildDashboardGenerator#generate() I'm rethrowing a possible
>> IOException by wrapping it in a RuntimeException, is this the right approach?
>
> I've added a comment to the commit.
>
>>
>> - As per Luke's suggestion I'm using JATL to generate report HTML.
>>
>> - I'm using Jerry to parse and search through the report HTML in tests,
>> Jerry allows jQuery like selectors on HTML in Java
>> (http://jodd.org/doc/jerry/index.html).
>>
>> - Report is generated under 'buildDashboard/index.html' relatively to
>> baseDir of ReportingExtension.
>
> This is all good.
>
>
> --
> Adam Murdoch
> Gradle Co-founder
> http://www.gradle.org
> VP of Engineering, Gradleware Inc. - Gradle Training, Support, Consulting
> http://www.gradleware.com
>
