aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Maxim Khutornenko" <ma...@apache.org>
Subject Re: Review Request 28914: Adding PMD rule to check @Timed annotation placement.
Date Wed, 10 Dec 2014 22:37:01 GMT


> On Dec. 10, 2014, 8:31 p.m., Kevin Sweeney wrote:
> > config/pmd/custom.xml, line 21
> > <https://reviews.apache.org/r/28914/diff/3/?file=788487#file788487line21>
> >
> >     Name = Aurora?

Sure.


> On Dec. 10, 2014, 8:31 p.m., Kevin Sweeney wrote:
> > config/pmd/custom.xml, line 15
> > <https://reviews.apache.org/r/28914/diff/3/?file=788487#file788487line15>
> >
> >     Remove this comment.

Removed.


> On Dec. 10, 2014, 8:31 p.m., Kevin Sweeney wrote:
> > config/pmd/custom.xml, line 36
> > <https://reviews.apache.org/r/28914/diff/3/?file=788487#file788487line36>
> >
> >     Used with the @Timed annotation. Also consider adding a link to the guice docs
on method interceptor limitations for those unfamiliar.

Added.


> On Dec. 10, 2014, 8:31 p.m., Kevin Sweeney wrote:
> > config/pmd/custom.xml, line 43
> > <https://reviews.apache.org/r/28914/diff/3/?file=788487#file788487line43>
> >
> >     My XPath-foo is lacking - will this catch all of the following forms:
> >     
> >     ```java
> >     @Timed("test_var")
> >     private void myMethod() {}
> >     
> >     @Timed
> >     void myMethod2() {}
> >     
> >     @Timed(value = "time_var")
> >     static void myMethod3() {}
> >     ```
> >     
> >     Also is there a way to limit this match to the Timed annotation from com.twitter.common.inject.TimedInterceptor.Timed?

Modifed to support all use cases.

As for the type resolution, this would be much more involved and would require implementing
part of the rule in java to use ClassLoader. I am punting on this.


- Maxim


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


On Dec. 10, 2014, 8:06 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28914/
> -----------------------------------------------------------
> 
> (Updated Dec. 10, 2014, 8:06 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Bill Farner.
> 
> 
> Bugs: AURORA-967
>     https://issues.apache.org/jira/browse/AURORA-967
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Adding PMD rule to check @Timed annotation placement.
> 
> 
> Diffs
> -----
> 
>   config/pmd/custom.xml PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/28914/diff/
> 
> 
> Testing
> -------
> 
> ```
> $ ./gradlew -Pq --rerun-tasks pmdMain
> :buildSrc:compileJava UP-TO-DATE
> :buildSrc:compileGroovy
> :buildSrc:processResources UP-TO-DATE
> :buildSrc:classes
> :buildSrc:jar
> :buildSrc:assemble
> :buildSrc:compileTestJava UP-TO-DATE
> :buildSrc:compileTestGroovy UP-TO-DATE
> :buildSrc:processTestResources UP-TO-DATE
> :buildSrc:testClasses UP-TO-DATE
> :buildSrc:test UP-TO-DATE
> :buildSrc:check UP-TO-DATE
> :buildSrc:build
> :pmdMain
> /Users/mkhutornenko/workspace3/aurora/src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java:204:
A method must be overridable to have a @Timed annotation.
> :pmdMain FAILED
> 
> FAILURE: Build failed with an exception.
> 
> * What went wrong:
> Execution failed for task ':pmdMain'.
> > 1 PMD rule violations were found. See the report at: file:///Users/mkhutornenko/workspace3/aurora/dist/reports/pmd/main.html
> 
> * Try:
> Run with --stacktrace option to get the stack trace. Run with --info or --debug option
to get more log output.
> 
> BUILD FAILED
> 
> Total time: 12.981 secs
> ```
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message