drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Edmon Begoli <ebeg...@gmail.com>
Subject Re: Maven build failing on checkstyle
Date Thu, 10 Sep 2015 00:26:37 GMT
Ted et al.,

I created an issue as a "wish" for possible loosening up of the checkstyle.

https://issues.apache.org/jira/browse/DRILL-3756

On Wed, Sep 9, 2015 at 2:42 PM, Edmon Begoli <ebegoli@gmail.com> wrote:

> I will do whatever your suggest. I stripped spaces and re-comitted and
> merged into pull request.
>
> I do think that the stylechecker is being bit draconian, but I can live
> with it.
> I was just surprised to see build fail on such a small source commenting
> issue that is hard to detect for those not using IDE, as this vertically
> formatted javadoc is auto-inserted by the tool.
>
> However - I think having checkstyle like this does contribute to better
> code quality.
>
> I would also recommend having a checkstyle looking for something
> meaningful to code quality such as the presence of javadoc comments on
> public mehtods, empty javadoc comments, etc. etc.
>
>
> I will be happy to file JIRA, and you guys decide if it is worth acting on
> it or closing it. I am here to help the project, not to complain :-)
>
>
> On Wed, Sep 9, 2015 at 2:23 PM, Jim Scott <jscott@maprtech.com> wrote:
>
>> I would suggest not changing the checkstyle and having anyone wanting to
>> commit code back to configure their IDE's to just strip spaces at the ends
>> of lines. Every major IDE has supported this for nearly 10 years.
>>
>> On Wed, Sep 9, 2015 at 1:17 PM, Ted Dunning <ted.dunning@gmail.com>
>> wrote:
>>
>> > Checkstyle is clearly being too picky here.
>> >
>> > The only problem with spaces at the end of a line is that some tools
>> strip
>> > them out automagically.  This leads to format changes that make reviews
>> > (very slightly) more difficult.
>> >
>> > I would be willing to fix the checkstyle profile to be less draconian if
>> > you would be willing to file the JIRA.
>> >
>> >
>> >
>> > On Wed, Sep 9, 2015 at 5:14 AM, Edmon Begoli <ebegoli@gmail.com> wrote:
>> >
>> > > and I am sorry to bug you with this but to me, this was a prefectly
>> > > formatted javadoc and I was surprised to see build failing on it:
>> > >
>> > > /** Abstract class for StorePlugin implementations.
>> > >  * See StoragePlugin for description of the interface intent and its
>> > > methods.
>> > >  */
>> > > public abstract class AbstractStoragePlugin implements StoragePlugin{
>> > >   static final org.slf4j.Logger logger =
>> > > org.slf4j.LoggerFactory.getLogger(AbstractStoragePlugin.class);
>> > >
>> > > However, it had a space before the end of the line first line, and
>> > > checkstyle did not like it. I was using vim, not IDE.
>> > >
>> > > I am switching to IDEA ...
>> > >
>> > >
>> > > On Tue, Sep 8, 2015 at 11:48 PM, Edmon Begoli <ebegoli@gmail.com>
>> wrote:
>> > >
>> > > > I am running build on my fork, and Maven build is failing on the
>> > > > checkstyle:
>> > > >
>> > > > excerpt ...
>> > > >
>> > > > [INFO] --- maven-checkstyle-plugin:2.12.1:check
>> > (checkstyle-validation) @
>> > > > drill-java-exec ---
>> > > >
>> > > > [INFO] Starting audit...
>> > > >
>> > > >
>> > >
>> >
>> /Users/ebegoli/drill/exec/java-exec/src/main/java/org/apache/drill/exec/store/AbstractStoragePlugin.java:31:
>> > > > Line matches the illegal pattern '\s+$'.
>> > > >
>> > > >
>> > >
>> >
>> /Users/ebegoli/drill/exec/java-exec/src/main/java/org/apache/drill/exec/store/AbstractStoragePlugin.java:33:
>> > > > Line matches the illegal pattern '\s+$'.
>> > > >
>> > > >
>> > >
>> >
>> /Users/ebegoli/drill/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/easy/EasyFormatPlugin.java:118:
>> > > > Line matches the illegal pattern '\s+$'.
>> > > >
>> > > >
>> > >
>> >
>> /Users/ebegoli/drill/exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePlugin.java:30:
>> > > > Line matches the illegal pattern '\s+$'.
>> > > >
>> > > >
>> > >
>> >
>> /Users/ebegoli/drill/exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePlugin.java:35:
>> > > > Line matches the illegal pattern '\s+$'.
>> > > >
>> > > >
>> > >
>> >
>> /Users/ebegoli/drill/exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePlugin.java:44:
>> > > > Line matches the illegal pattern '\s+$'.
>> > > >
>> > > >
>> > >
>> >
>> /Users/ebegoli/drill/exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePlugin.java:45:
>> > > > Line matches the illegal pattern '\s+$'.
>> > > >
>> > > >
>> > >
>> >
>> /Users/ebegoli/drill/exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePlugin.java:71:
>> > > > Line matches the illegal pattern '\s+$'.
>> > > >
>> > > >
>> > >
>> >
>> /Users/ebegoli/drill/exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePlugin.java:74:
>> > > > Line matches the illegal pattern '\s+$'.
>> > > >
>> > > > Audit done.
>> > > >
>> > > > It looks like Javadoc checkstyle if failing. These are included in
>> my
>> > > pull:
>> > > >
>> > > > https://github.com/apache/drill/pull/139
>> > > >
>> > > >
>> > > > Can someone please advise how do I and should I either suppress
>> these
>> > or
>> > > > fix the issue.
>> > > >
>> > > > It is a properly structured javadoc. Starts with /** and ends with
>> */.
>> > > >
>> > > > Not sure what else is required, but I will happy to fix it to make
>> it
>> > > pass
>> > > > the checkstyle.
>> > > >
>> > > >
>> > > >
>> > > >
>> > > >
>> > > >
>> > > >
>> > >
>> >
>>
>>
>>
>> --
>> *Jim Scott*
>> Director, Enterprise Strategy & Architecture
>> +1 (347) 746-9281
>> @kingmesal <https://twitter.com/kingmesal>
>>
>> <http://www.mapr.com/>
>> [image: MapR Technologies] <http://www.mapr.com>
>>
>> Now Available - Free Hadoop On-Demand Training
>> <
>> http://www.mapr.com/training?utm_source=Email&utm_medium=Signature&utm_campaign=Free%20available
>> >
>>
>
>

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