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 > >> > > > > >
