[
https://issues.apache.org/jira/browse/CASSANDRA-17925?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17640223#comment-17640223
]
Stefan Miklosovic commented on CASSANDRA-17925:
-----------------------------------------------
That works for me but I am not sure if it is possible to just warn and not
error out in practice. Do you mean you want to warn just on new / modified
files? So in that case, checkstyle would have to know what "new or modified"
means. I think that checkstyle can not see this. If it was just warning all
such violations, the output would be monstrously long but I guess that is fine?
How do we motivate people to actually fix it though.
We might also wait for big integrations to be merged and we do the big-bang
after that.
> Java source code should have sorted imports as defined in the codestyle
> -----------------------------------------------------------------------
>
> Key: CASSANDRA-17925
> URL: https://issues.apache.org/jira/browse/CASSANDRA-17925
> Project: Cassandra
> Issue Type: Improvement
> Reporter: Stefan Miklosovic
> Assignee: Maxim Muzafarov
> Priority: Normal
>
> After we cleaned all unused imports in CASSANDRA-17876, there is one more
> task remaining to be done - to add checkstyle for imports order and enforce
> this on build time.
> When the project is imported into IDEA, there is a helper target on Ant
> called "generate-idea-files". ide/idea/codeStyleSettings.xml contains this:
> {code:java}
> <option name="IMPORT_LAYOUT_TABLE">
> <value>
> <package name="java" withSubpackages="true" static="false" />
> <package name="javax" withSubpackages="true" static="false" />
> <emptyLine />
> <package name="com.google.common" withSubpackages="true"
> static="false" />
> <package name="org.apache.log4j" withSubpackages="true"
> static="false" />
> <package name="org.apache.commons" withSubpackages="true"
> static="false" />
> <package name="org.cliffc.high_scale_lib" withSubpackages="true"
> static="false" />
> <package name="org.junit" withSubpackages="true" static="false" />
> <package name="org.slf4j" withSubpackages="true" static="false" />
> <emptyLine />
> <package name="" withSubpackages="true" static="false" />
> <emptyLine />
> <package name="" withSubpackages="true" static="true" />
> </value>
> </option>
> {code}
> This code style is also mentioned in the web page here (minus some details
> which are present in above configuration snippet but not on the web page):
> [https://cassandra.apache.org/_/development/code_style.html] (at the very
> bottom).
> However, when one runs "Optimise imports" in the context menu after
> right-clicking on org.cassandra.apache package, it will refactor the imports
> and it results with hundreds of changes.
> This means that the source code, as-is, does not adhere to the self-imposed
> code style we ship for IDEA.
> If we fix this, we should add checkstyle for it like this:
> {code:java}
> <module name="ImportOrder">
> <property name="groups"
> value="/(^java\.|javax)/,/(com\.google\.common|org\.apache\.log4j|org\.apache\.commons|org\.cliffc\.high_scale_lib|org\.junit|org\.slf4j)/"/>
> <property name="ordered" value="true"/>
> <property name="separated" value="true"/>
> <property name="option" value="bottom"/>
> <property name="separatedStaticGroups" value="false"/>
> <property name="sortStaticImportsAlphabetically" value="true"/>
> </module>
> {code}
> This checkstyle on import order will pass on the source code we run the
> import optimization in the context menu on.
> There is also no enforcement on "all star" imports (org.some.pkg.*).
> Checkstyle has specific module for this:
> [https://checkstyle.org/config_imports.html#AvoidStarImport]
> I propose we should stop to use all-star imports. Same argument holds as
> described there: Rationale: Importing all classes from a package or static
> members from a class leads to tight coupling between packages or classes and
> might lead to problems when a new version of a library introduces name
> clashes.
> This should be applied to test checktyle as well and the source code should
> be refactored on imports too.
> This should be done on cassandra-4.1 as well as for trunk.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
