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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> > 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 > > >
