Hi Dongjin, Thanks for bringing this up. I’m in favor of adding a check style rule.
Thanks for testing out the alternatives. As long as we aren’t adding wildcard imports and as long as it’s easy to configure IDEs with our chosen rules, I have no preference. I assume these hold for your proposals, since we’d otherwise have a huge number of changes files. I guess the options with the least changes is likely to be the one for which the fewest people would have to change IDE settings? Thanks, John On Fri, Oct 23, 2020, at 11:44, Sophie Blee-Goldman wrote: > Bruno is committer-like :) > > I generally prefer option 2, but would definitely be happy with any of them. > > I'm pretty sure I'm personally responsible for some of the wacky import > ordering way back when, before I set my IDE configuration straight. It took > me a while to notice because we never had a checkstyle rule for import > order. > So thank you for proposing this. > > Sophie > > On Thu, Oct 22, 2020 at 11:17 PM Bruno Cadonna <br...@confluent.io> wrote: > > > Hi Dongjin, > > > > Thank you that you put me into the committer section, but I am actually > > not a committer. > > > > Best, > > Bruno > > > > On 23.10.20 07:46, Dongjin Lee wrote: > > > As of Present: > > > > > > Committers: > > > > > > - Bruno: 2 and 3. > > > - Gwen: (No Specific Preference) > > > > > > Non-Committers: > > > > > > - Brandon: 2. > > > - Dongjin: 2 and 3. > > > > > > Let's hold on for 2 or 3 committers. > > > > > > Best, > > > Dongjin > > > > > > On Fri, Oct 23, 2020 at 10:09 AM Gwen Shapira <g...@confluent.io> wrote: > > > > > >> I don't have any specific preference on the style. But I am glad you > > >> are bringing it up. Every other project I worked on had a specific > > >> import style, and the random import changes in PRs are pretty > > >> annoying. > > >> > > >> On Wed, Oct 14, 2020 at 10:36 PM Dongjin Lee <dong...@apache.org> > > wrote: > > >>> > > >>> Hello. I hope to open a discussion about the import order in Java code. > > >>> > > >>> As Nikolay stated recently[^1], Kafka uses a relatively strict code > > style > > >>> for Java code. However, it misses any rule on import order. For this > > >>> reason, the code formatting settings of every local dev environment are > > >>> different from person to person, resulting in the countless meaningless > > >>> import order changes in the PR. > > >>> > > >>> For example, in `NamedCache.java` in the streams module, the `java.*` > > >>> imports are split into two chunks, embracing the other imports between > > >>> them. So, I propose to define an import order to prevent these kinds of > > >>> cases in the future. > > >>> > > >>> To define the import order, we have to regard the following three > > >>> orthogonal issues beforehand: > > >>> > > >>> a. How to group the type imports? > > >>> b. Whether to sort the imports alphabetically? > > >>> c. Where to place static imports: above the type imports, or below > > them. > > >>> > > >>> Since b and c seem relatively straightforward (sort the imports > > >>> alphabetically and place the static imports below the type imports), I > > >> hope > > >>> to focus the available alternatives on the problem a. > > >>> > > >>> I evaluated the following alternatives and checked how many files are > > get > > >>> effected for each case. (based on commit 1457cc652) And here are the > > >>> results: > > >>> > > >>> *1. kafka, org.apache.kafka, *, javax, java (5 groups): 1222 files.* > > >>> > > >>> ``` > > >>> <module name="ImportOrder"> > > >>> <property name="groups" > > >>> value="kafka,/^org\.apache\.kafka.*$/,*,javax,java"/> > > >>> <property name="ordered" value="true"/> > > >>> <property name="separated" value="true"/> > > >>> <property name="option" value="bottom"/> > > >>> <property name="sortStaticImportsAlphabetically" value="true"/> > > >>> </module> > > >>> ``` > > >>> > > >>> *2. (kafka|org.apache.kafka), *, javax? (3 groups): 968 files.* > > >>> > > >>> ``` > > >>> <module name="ImportOrder"> > > >>> <property name="groups" > > >> value="(kafka|org\.apache\.kafka),*,javax?"/> > > >>> <property name="ordered" value="true"/> > > >>> <property name="separated" value="true"/> > > >>> <property name="option" value="bottom"/> > > >>> <property name="sortStaticImportsAlphabetically" value="true"/> > > >>> </module> > > >>> ``` > > >>> > > >>> *3. (kafka|org.apache.kafka), *, javax, java (4 groups): 533 files.* > > >>> > > >>> ``` > > >>> <module name="ImportOrder"> > > >>> <property name="groups" > > >>> value="(kafka|org\.apache\.kafka),*,javax,java"/> > > >>> <property name="ordered" value="true"/> > > >>> <property name="separated" value="true"/> > > >>> <property name="option" value="bottom"/> > > >>> <property name="sortStaticImportsAlphabetically" value="true"/> > > >>> </module> > > >>> ``` > > >>> > > >>> *4. *, javax? (2 groups): 707 files.* > > >>> > > >>> ``` > > >>> <module name="ImportOrder"> > > >>> <property name="groups" value="*,javax?"/> > > >>> <property name="ordered" value="true"/> > > >>> <property name="separated" value="true"/> > > >>> <property name="option" value="bottom"/> > > >>> <property name="sortStaticImportsAlphabetically" value="true"/> > > >>> </module> > > >>> ``` > > >>> > > >>> *5. javax?, * (2 groups): 1822 files.* > > >>> > > >>> ``` > > >>> <module name="ImportOrder"> > > >>> <property name="groups" value="javax?,*"/> > > >>> <property name="ordered" value="true"/> > > >>> <property name="separated" value="true"/> > > >>> <property name="option" value="bottom"/> > > >>> <property name="sortStaticImportsAlphabetically" value="true"/> > > >>> </module> > > >>> ``` > > >>> > > >>> *6. java, javax, * (3 groups): 1809 files.* > > >>> > > >>> ``` > > >>> <module name="ImportOrder"> > > >>> <property name="groups" value="java,javax,*"/> > > >>> <property name="ordered" value="true"/> > > >>> <property name="separated" value="true"/> > > >>> <property name="option" value="bottom"/> > > >>> <property name="sortStaticImportsAlphabetically" value="true"/> > > >>> </module> > > >>> ``` > > >>> > > >>> I hope to get some feedback on this issue here. > > >>> > > >>> For the WIP PR, please refer here: > > >> https://github.com/apache/kafka/pull/8404 > > >>> > > >>> Best, > > >>> Dongjin > > >>> > > >>> [^1]: > > >>> > > >> > > https://lists.apache.org/thread.html/r2bbee24b8a459842a0fc840c6e40958e7158d29f3f2d6c0d223be80b%40%3Cdev.kafka.apache.org%3E > > >>> [^2]: > > >>> > > >> > > https://github.com/apache/kafka/blob/trunk/streams/src/main/java/org/apache/kafka/streams/state/internals/NamedCache.java > > >>> > > >>> -- > > >>> *Dongjin Lee* > > >>> > > >>> *A hitchhiker in the mathematical world.* > > >>> > > >>> > > >>> > > >>> > > >>> *github: <http://goog_969573159/>github.com/dongjinleekr > > >>> <https://github.com/dongjinleekr>keybase: > > >> https://keybase.io/dongjinleekr > > >>> <https://keybase.io/dongjinleekr>linkedin: > > >> kr.linkedin.com/in/dongjinleekr > > >>> <https://kr.linkedin.com/in/dongjinleekr>speakerdeck: > > >> speakerdeck.com/dongjin > > >>> <https://speakerdeck.com/dongjin>* > > >> > > >> > > >> > > >> -- > > >> Gwen Shapira > > >> Engineering Manager | Confluent > > >> 650.450.2760 | @gwenshap > > >> Follow us: Twitter | blog > > >> > > > > > > > > >
