Also I might want to add check style for Apache header (which should be in a form of comment, not Javadoc), agreed code style (like indents etc) and enforced java doc for methods. At least the last two are enforced in Calcite. I used to point to all that stuff during code reviews but if all that would be enforced, it would be much easier ...
Kind regards Arina On Sat, Sep 9, 2017 at 6:46 AM, Paul Rogers <[email protected]> wrote: > Hi Vlad, > > Java has a wide variety of warnings available; each project decides which > to ignore, which are warnings and which are errors. It may be that Eclipse, > by default, has resource warnings turned on. The quick & dirty solution is > simply to turn off warnings for AutoCloseables and missing @Overrides. This > is, as they say, “crude but effective." > > It seems that the Drill community stand on imports is not to change them. > Eclipse has an “organize imports” feature. I have to be careful when > removing unused imports not to invoke this feature as it changes import > order and often cause reviews to complain about unnecessary code changes. > > Would be good if we could 1) agree on a standard and 2) make sure that > both Eclipse and IntelliJ can automatically organize imports to follow the > standard. But, I personally don’t worry about imports because Eclipse takes > care of it for me. > > For me, the bigger concern is about code style. Operators are implemented > as huge, complex, deeply nested methods with many local variables (such as > flags) set one place and used elsewhere — all with no comments. Would seem > like a good idea to adopt best practices and require human-digestible > method sizes with good Javadoc comments. To my mind, that will contribute > more to the project than import order. > > Oh, and the other item that needs addressing is a requirement to create > true unit tests (not just system tests coded with JUnit.) Good unit test > will increase our code quality immensely, and will simplify the task for > code reviews. So, I’d want to push that ahead before worrying about imports. > > Just my two cents… > > Thanks, > > - Paul > > > On Sep 8, 2017, at 6:58 PM, Vlad Rozov <[email protected]> wrote: > > > > Paul, is AutoCloseable warning specific to Eclipse? I don't remember > seeing the same warning in IntelliJ or during compilation. > > > > I know that some communities are significantly more strict regarding > code style and enforce not only unused imports, but also order of imports > and placement of static imports. What is the Drill community stand on those > items? > > > > Thank you, > > > > Vlad > > > > On 9/8/17 18:04, Paul Rogers wrote: > >> I clean up the imports as I find them, but it would be nice to do them > all at once to avoid the constant drip-drip-drop of warnings. > >> > >> The key problem is the generated code: the templates can’t really tell > which imports are used where. So, we’d need to exclude generated code > directories from the check style rules. > >> > >> Drill also has thousands of omitted “@Override” annotations and heavy > abuse of AutoCloseable (which triggers warnings when used outside of > try-with-resources). > >> > >> At present, Eclipse complains about 17,883 warnings in Drill code. > >> > >> - Paul > >> > >>> On Sep 8, 2017, at 4:43 PM, Timothy Farkas <[email protected]> wrote: > >>> > >>> Hi All, > >>> > >>> I've noticed that a lot of files have unused imports, and I frequently > accidentally leave unused imports behind when I do refactoring. So I'd like > to enable checkstyle to check for unused imports. > >>> > >>> Thanks, > >>> Tim > > > >
