aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Kevin Sweeney" <kevi...@apache.org>
Subject Re: Review Request 17717: License header cleanup introduced in a584410c.
Date Wed, 05 Feb 2014 20:11:38 GMT


> On Feb. 5, 2014, 9:47 a.m., Kevin Sweeney wrote:
> > src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java, line
1
> > <https://reviews.apache.org/r/17717/diff/3/?file=470163#file470163line1>
> >
> >     same as above
> 
> Jake Farrell wrote:
>     All existing license headers within the project are using JAVADOC_STYLE (/**), behind
the scenes the gradle license plugin uses the maven license plugin which defaults the comment
style for java type files to JAVADOC_STYLE. This format covers *.java, *.groovy, *.css, *.cs,
*.as, *.aj, *.c, *.h, *.cpp.
>     
>     We can override this and switch it to mapping('java','SLASHSTAR_STYLE'), but I think
we would want to keep the defaults and use JAVADOC_STYLE since other projects use these plugins
and will most likely default to this style as well. Another reason to keep the JAVADOC_STYLE
is that it looks like intellij #parse directive for automatically adding templated license
headers defaults to JAVADOC_STYLE as well. 
>     
>     Drawbacks I can quickly think of for switching to SLASHSTAR_STYLE will be: 
>     - we will have to custom override for any filetypes defaults used in the project
>     - any generated code from Thrift will contain the JAVADOC_STYLE and checkstyle will
error out (could make the regex check optional on the second * but that could lead to JAVADOC_STYLE
making it back into src)
>     - every existing license header in the project would have to be converted over to
the SLASHSTAR_STYLE
>     
>     I dont have a strong objection to switching to SLASHSTAR_STYLE on this, just think
it might be better to keep with JAVADOC_STYLE since its the default over adding customizations,
thought being so we can be more inline for using with other plugins/checks/etc that would
be expecting the defaulted style. 
>
> 
> Bill Farner wrote:
>     > These should be single *, otherwise they're rendered as part of the class javadoc
>     
>     This is false, both intellij and the javadoc tool render the javadoc associated with
the class, which will be the block immediately before the class signature.  Source: i viewed
javadoc in intellij and ran the javadoc tool on a class with the javadoc-style header.

Thanks for checking on that Bill, that addresses my concern.


- Kevin


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


On Feb. 5, 2014, 9:45 a.m., Jake Farrell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17717/
> -----------------------------------------------------------
> 
> (Updated Feb. 5, 2014, 9:45 a.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney, Mark Chu-Carroll, and Bill Farner.
> 
> 
> Bugs: AURORA-193
>     https://issues.apache.org/jira/browse/AURORA-193
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> License header cleanup introduced in a584410c.
> 
> 
> Diffs
> -----
> 
>   build-support/thrift/Makefile c009890465323c022596aab84553bd00659dba9b 
>   build.gradle 2f355d3528fb4fd5d41ce021ae3330b3af9142f5 
>   config/checkstyle/apache.header PRE-CREATION 
>   config/checkstyle/apache.header.regex PRE-CREATION 
>   config/checkstyle/checkstyle.xml 071e40c52c4f770902fb1c79002198ef35f2cf41 
>   src/main/java/org/apache/aurora/scheduler/events/EventSink.java 4fc425e8a3d6d5931c8923fcca1f4d1d51451dab

>   src/test/java/org/apache/aurora/scheduler/async/RescheduleCalculatorImplTest.java 70eb52a3a44f3c6c0bde3583fca492785faf93d0

>   src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 940dac4e47bd19e48e8226def245a53ad6f19710

> 
> Diff: https://reviews.apache.org/r/17717/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jake Farrell
> 
>


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