4 I would like to avoid star imports in general. Having them explicitly enumerated seems to be better, it can not happen we use some import wrongly (by accident). There is rational behind this here (1).
This check can be configured in such a way that star imports could be possible on some packages. For example "org.apache.cassandra.tools.NodeTool" has star import "import org.apache.cassandra.tools.nodetool.*;" where all nodetool commands are. This can be configured so it would remain to be star import on that package so we do not need to explicitly import 100+ classes. There might be more cases like this but they would need to be reviewed more closely. (1) https://checkstyle.org/config_imports.html#AvoidStarImport ________________________________________ From: Maxim Muzafarov <mmu...@apache.org> Sent: Thursday, December 22, 2022 15:52 To: dev@cassandra.apache.org Subject: Re: [DISCUSSION] Cassandra's code style and source code analysis NetApp Security WARNING: This is an external email. Do not click links or open attachments unless you recognize the sender and know the content is safe. Hello everyone, have a great vacation and happy holidays to all! I've completed a small research about how the classe's import order rule are spread in the Apache projects. Some of the projects don't have any restrictions over the imports even if they are using the checkstyle configuration. The other ones may have only the consensus over the imports, but they are not reflected in the checkstyle yet (e.g. Kafka). The conclusion here can only be that there is a very large variability in the classe's import order, so we have to agree on the order on our own. You can find the projects, IDEs and frameworks and their corresponding classe's import order below: https://mmuzaf.github.io/blog/Java_Import_Orders.html Most of the time during development in an IDE the classe's imports remains collapsed, so from my point of view the following things related to the classe's import comes into the first place to consider: - a PR review: newly imports must be clearly visible; - try to minimize the total amount of affected files; - the import order rule must be implemented in a simple way and well supported by IDEs and its plugins; In addition to the last mentioned option, the checkstyle itself has some limitations also. For instance, the ImportOrder has a limitation by design to enforce an empty line between groups ("java", "javax"), or CustomImportOrder may have only up to 4 custom groups separated by a blank line. Based on all of the above I can propose the following classe's order. All of them are tested on the latest changes from the trunk branch (commit hash: b171b4ba294126e985d0ee629744516f19c8644e) 1. Total 2 groups, 3072 files to change ``` all other imports [blank line] static all other imports ``` 2. Total 3 groups, 2345 files to change ``` java.* javax.* [blank line] all other imports [blank line] static all other imports ``` 3. Total 5 groups, 2968 files to change ``` org.apache.cassandra.* [blank line] java.* [blank line] javax.* [blank line] all other imports [blank line] static all other imports ``` 4. Total 5 groups, 1792 files to change ``` java.* javax.* [blank line] com.* net.* org.* [blank line] org.apache.cassandra.* [blank line] all other imports [blank line] static all other imports ``` 5. Total 2 groups, 3114 files to change ``` java.* javax.* org.apache.cassandra.* all other imports [blank line] static all other imports ``` Of course, any suggestions are really appreciated. Please, share your thoughts. On Thu, 15 Dec 2022 at 17:48, Mick Semb Wever <m...@apache.org> wrote: >> >> Another angle I forgot to mention is that this is quite a big patch and >> there are quite big pieces of work coming, being it CEP-15, for example. So >> I am trying to figure out if we are ok to just merge this work first and >> devs doing CEP-15 will need to rework their imports or we merge this after >> them so we will fix their stuff. I do not know what is more preferable. > > > > Thank you for bringing this point up Stefan. > > I would be actively reaching out to all those engaged with current CEPs, > asking them the rebase impact this would cause and if they are ok with it. > The CEPs are our priority, and we have a significant amount of them in > progress compared to anything we've had for many years. > > >
