Mailing-List: contact reviews-help@aurora.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: reviews@aurora.incubator.apache.org
Content-Type: multipart/alternative;
 boundary="===============7232400212247311914=="
MIME-Version: 1.0
Subject: Re: Review Request 21455: Opt-in for code quality checks to speed up
 development iteration.
From: "Bill Farner" <wfarner@apache.org>
To: "Maxim Khutornenko" <maxim@apache.org>
Cc: "Bill Farner" <wfarner@apache.org>,
 "Aurora" <reviews@aurora.incubator.apache.org>,
 "Jake Farrell" <jfarrell@apache.org>
Date: Tue, 20 May 2014 17:36:42 -0000
Message-ID: <20140520173642.21125.66982@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: "Bill Farner" <noreply@reviews.apache.org>
References: <20140515230431.715.44662@reviews.apache.org>
In-Reply-To: <20140515230431.715.44662@reviews.apache.org>
Reply-To: "Bill Farner" <wfarner@apache.org>

--===============7232400212247311914==
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit


> On May 15, 2014, 11:04 p.m., Maxim Khutornenko wrote:
> > build.gradle, line 235
> > <https://reviews.apache.org/r/21455/diff/1/?file=581766#file581766line235>
> >
> >     How about a custom target instead? Having something like "gradle build_cq" would be easier to remember for use outside of jenkins script.
> 
> Jake Farrell wrote:
>     agree, think having these tied to a task would be better, maybe gradle verify since check is part of the lifecycle all ready
> 
> Bill Farner wrote:
>     Do either of you know how to express that?
>     
>     I see 2 fundamental approaches:
>     
>     - dynamically remove verification tasks when 'build' is being run
>     
>     This seems like an uphill battle, since we would need to know which tasks the plugins are adding dependencies to, and remove them.  Removing task dependencies also seems to be frowned upon [1].
>     
>     [1] http://forums.gradle.org/gradle/topics/how_to_remove_a_task_dependency
>     
>     - conditionally enable verification tasks when the 'verify' task is being run
>     
>     The only implementation i have come up with is to figure out which task gradle was invoked with, and set a flag if 'verify' was invoked.  This has shortcomings in that the 'verify' task cannot be extended with other tasks.
>     
>     
>     Overall, both of the above approaches seem to go against the grain of how gradle is intended to work.  Are there other approaches you guys are thinking of?
> 
> Maxim Khutornenko wrote:
>     The 'verify' inheritance shortcoming does not sound like a deal breaker. I doubt the ability to extend it further would be vital (unless I am missing something).
>     
>     If none of the above work any chance the '-PrunCodeQuality=true' could be reduced to something shorter and easier to remember? I.e. a single letter option '-q'?

> The 'verify' inheritance shortcoming does not sound like a deal breaker.

If that were the only downside, i'd probably agree with you.  But the deal breaker comes with the additional downsides of dependency removal being frowned upon by gradle developers, and the fact that we have to hunt through the dependency graph to find where these tasks are attaching.  I agree that bundling this in a task is nice, but it feels like we might be asking for more tooling battles if we take this approach.

> could be reduced to something shorter and easier to remember? I.e. a single letter option '-q'

Not that i know of, have you found anything to suggest that you can?


- Bill


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21455/#review43177
-----------------------------------------------------------


On May 14, 2014, 8:31 p.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21455/
> -----------------------------------------------------------
> 
> (Updated May 14, 2014, 8:31 p.m.)
> 
> 
> Review request for Aurora and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Inclusion of findbugs has had the side-effect of slowing down development iteration.  It makes sense to only perform code quality checks on-demand, so we would typically do this in jenkins, before publishing a review diff, and before pushing to master.
> 
> 
> Diffs
> -----
> 
>   build-support/jenkins/build.sh f6f4940a1450cd0d8d8497e651d0e6c7377dfc3f 
>   build.gradle f2c729e28b01c89e2130c48d2aa43d7c830528a5 
> 
> Diff: https://reviews.apache.org/r/21455/diff/
> 
> 
> Testing
> -------
> 
> Before this change:
> 
> $ ./gradlew clean build
> :clean
> :about
> :bootstrapThrift UP-TO-DATE
> :generateSources
> :compileGeneratedJava
> :processGeneratedResources UP-TO-DATE
> :generatedClasses
> :compileJava
> :processResources
> :classes
> :jar
> :assemble
> :checkstyleMain
> :compileTestJava
> :processTestResources
> :testClasses
> :checkstyleTest
> :findbugsGenerated
> :findbugsMain
> :findbugsTest
> :licenseGenerated UP-TO-DATE
> :licenseMain UP-TO-DATE
> :licenseTest UP-TO-DATE
> :license UP-TO-DATE
> :test
> 
> BUILD SUCCESSFUL
> 
> Total time: 9 mins 5.6 secs
> 
> 
> After this change:
> $ ./gradlew clean build
> :clean
> :about
> :bootstrapThrift UP-TO-DATE
> :generateSources
> :compileGeneratedJava
> :processGeneratedResources UP-TO-DATE
> :generatedClasses
> :compileJava
> :processResources
> :classes
> :jar
> :assemble
> :checkstyleMain SKIPPED
> :compileTestJava
> :processTestResources
> :testClasses
> :checkstyleTest SKIPPED
> :findbugsGenerated SKIPPED
> :findbugsMain SKIPPED
> :findbugsTest SKIPPED
> :licenseGenerated SKIPPED
> :licenseMain SKIPPED
> :licenseTest SKIPPED
> :license UP-TO-DATE
> :test
> 
> BUILD SUCCESSFUL
> 
> Total time: 1 mins 41.661 secs
> 
> 
> /gradlew -PrunCodeQuality=true clean build
> Parallel execution is an incubating feature.
> :clean
> :about
> :bootstrapThrift UP-TO-DATE
> :generateSources
> :compileGeneratedJava
> :processGeneratedResources UP-TO-DATE
> :generatedClasses
> :compileJava
> :processResources
> :classes
> :jar
> :assemble
> :checkstyleMain
> :compileTestJava
> :processTestResources
> :testClasses
> :checkstyleTest
> :findbugsGenerated
> :findbugsMain
> :findbugsTest
> :licenseGenerated UP-TO-DATE
> :licenseMain UP-TO-DATE
> :licenseTest UP-TO-DATE
> :license UP-TO-DATE
> :test
> :jacocoTestReport
> :check
> :build
> 
> BUILD SUCCESSFUL
> 
> Total time: 6 mins 58.768 secs
> 
> 
> Thanks,
> 
> Bill Farner
> 
>


--===============7232400212247311914==--