You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Maxim Khutornenko <ma...@apache.org> on 2014/12/10 20:57:12 UTC

Review Request 28914: Adding PMD rule to check @Timed annotation placement.

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

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/design.xml 2741e407e35826e73bd4dc8335c0ef6294086398 

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

Re: Review Request 28914: Adding PMD rule to check @Timed annotation placement.

Posted by Maxim Khutornenko <ma...@apache.org>.

> On Dec. 10, 2014, 8:01 p.m., Kevin Sweeney wrote:
> > config/pmd/design.xml, line 1909
> > <https://reviews.apache.org/r/28914/diff/1/?file=788485#file788485line1909>
> >
> >     Also, consider adding this to a "custom.xml" file.

Good idea. Done.


- Maxim


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


On Dec. 10, 2014, 8:02 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:02 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/design.xml 2741e407e35826e73bd4dc8335c0ef6294086398 
> 
> 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
> 
>

Re: Review Request 28914: Adding PMD rule to check @Timed annotation placement.

Posted by Kevin Sweeney <ke...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28914/#review64618
-----------------------------------------------------------



config/pmd/design.xml
<https://reviews.apache.org/r/28914/#comment107396>

    Also, consider adding this to a "custom.xml" file.


- Kevin Sweeney


On Dec. 10, 2014, 11:57 a.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28914/
> -----------------------------------------------------------
> 
> (Updated Dec. 10, 2014, 11:57 a.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/design.xml 2741e407e35826e73bd4dc8335c0ef6294086398 
> 
> 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
> 
>

Re: Review Request 28914: Adding PMD rule to check @Timed annotation placement.

Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28914/#review64654
-----------------------------------------------------------

Ship it!


Master (2aac148) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot retry"

- Aurora ReviewBot


On Dec. 10, 2014, 10:37 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, 10:37 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
> 
>

Re: Review Request 28914: Adding PMD rule to check @Timed annotation placement.

Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28914/
-----------------------------------------------------------

(Updated Dec. 10, 2014, 10:37 p.m.)


Review request for Aurora, Kevin Sweeney and Bill Farner.


Changes
-------

CR comments.


Bugs: AURORA-967
    https://issues.apache.org/jira/browse/AURORA-967


Repository: aurora


Description
-------

Adding PMD rule to check @Timed annotation placement.


Diffs (updated)
-----

  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

Re: Review Request 28914: Adding PMD rule to check @Timed annotation placement.

Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28914/#review64623
-----------------------------------------------------------

Ship it!


Master (2aac148) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot retry"

- Aurora ReviewBot


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
> 
>

Re: Review Request 28914: Adding PMD rule to check @Timed annotation placement.

Posted by Maxim Khutornenko <ma...@apache.org>.

> On Dec. 10, 2014, 9:07 p.m., Bill Farner wrote:
> > config/pmd/design.xml, line 1923
> > <https://reviews.apache.org/r/28914/diff/2/?file=788486#file788486line1923>
> >
> >     Should this be >=1?  JDK8 supports multiple annotations of the same type, so while silly it might be good to keep in mind.

Sure, ">0" does the trick.


- Maxim


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


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
> 
>

Re: Review Request 28914: Adding PMD rule to check @Timed annotation placement.

Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28914/#review64631
-----------------------------------------------------------

Ship it!



config/pmd/design.xml
<https://reviews.apache.org/r/28914/#comment107416>

    Should this be >=1?  JDK8 supports multiple annotations of the same type, so while silly it might be good to keep in mind.


- Bill Farner


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
> 
>

Re: Review Request 28914: Adding PMD rule to check @Timed annotation placement.

Posted by Maxim Khutornenko <ma...@apache.org>.

> 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
> 
>

Re: Review Request 28914: Adding PMD rule to check @Timed annotation placement.

Posted by Kevin Sweeney <ke...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28914/#review64624
-----------------------------------------------------------



config/pmd/custom.xml
<https://reviews.apache.org/r/28914/#comment107411>

    Remove this comment.



config/pmd/custom.xml
<https://reviews.apache.org/r/28914/#comment107409>

    Name = Aurora?



config/pmd/custom.xml
<https://reviews.apache.org/r/28914/#comment107410>

    Used with the @Timed annotation. Also consider adding a link to the guice docs on method interceptor limitations for those unfamiliar.



config/pmd/custom.xml
<https://reviews.apache.org/r/28914/#comment107412>

    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?


- Kevin Sweeney


On Dec. 10, 2014, 12: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, 12: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
> 
>

Re: Review Request 28914: Adding PMD rule to check @Timed annotation placement.

Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
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.


Changes
-------

Kevin's comments.


Bugs: AURORA-967
    https://issues.apache.org/jira/browse/AURORA-967


Repository: aurora


Description
-------

Adding PMD rule to check @Timed annotation placement.


Diffs (updated)
-----

  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

Re: Review Request 28914: Adding PMD rule to check @Timed annotation placement.

Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28914/
-----------------------------------------------------------

(Updated Dec. 10, 2014, 8:02 p.m.)


Review request for Aurora, Kevin Sweeney and Bill Farner.


Changes
-------

Fixed description.


Bugs: AURORA-967
    https://issues.apache.org/jira/browse/AURORA-967


Repository: aurora


Description
-------

Adding PMD rule to check @Timed annotation placement.


Diffs (updated)
-----

  config/pmd/design.xml 2741e407e35826e73bd4dc8335c0ef6294086398 

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

Re: Review Request 28914: Adding PMD rule to check @Timed annotation placement.

Posted by Maxim Khutornenko <ma...@apache.org>.

> On Dec. 10, 2014, 8 p.m., Kevin Sweeney wrote:
> > config/pmd/design.xml, lines 1914-1915
> > <https://reviews.apache.org/r/28914/diff/1/?file=788485#file788485line1914>
> >
> >     Fix description?

Yup, just noticed and updated.


- Maxim


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


On Dec. 10, 2014, 8:02 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:02 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/design.xml 2741e407e35826e73bd4dc8335c0ef6294086398 
> 
> 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
> 
>

Re: Review Request 28914: Adding PMD rule to check @Timed annotation placement.

Posted by Kevin Sweeney <ke...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28914/#review64617
-----------------------------------------------------------



config/pmd/design.xml
<https://reviews.apache.org/r/28914/#comment107395>

    Fix description?


- Kevin Sweeney


On Dec. 10, 2014, 11:57 a.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28914/
> -----------------------------------------------------------
> 
> (Updated Dec. 10, 2014, 11:57 a.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/design.xml 2741e407e35826e73bd4dc8335c0ef6294086398 
> 
> 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
> 
>

Re: Review Request 28914: Adding PMD rule to check @Timed annotation placement.

Posted by Kevin Sweeney <ke...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28914/#review64616
-----------------------------------------------------------

Ship it!


Ship It!

- Kevin Sweeney


On Dec. 10, 2014, 11:57 a.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28914/
> -----------------------------------------------------------
> 
> (Updated Dec. 10, 2014, 11:57 a.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/design.xml 2741e407e35826e73bd4dc8335c0ef6294086398 
> 
> 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
> 
>