As of Present (Corrected and Updated): Committers:
- Gwen, John, Matthias: (No Specific Preference) - Sophie: 2. Non-Committers: - Brandon: 2. - Bruno: 2 and 3. - Dongjin: 2 and 3. It seems like option 2 (i.e., the second least changing, three group scheme of kafka, java, etc.) is the most favorable of the community. > 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? (John) Probably. In fact, it was the first presumption I had when I started this work. But on second thought, I noticed conciseness should be regarded also. It is why I tested and provided the alternatives - and yes, option 2 could take both conciseness and least changing. > My main ask is, can we document clearly, how tools like IntelliJ and Eclipse need to be configured such that they obey the defined rules? It would be very annoying of it would be required to disable "auto import" and manually write import statements to obey the rules. (Matthias) Of course, I am planning to not only document how to set up the tools[^1] but also add a Gradle plugin[^2] to enforce the formatting. With this configuration, Gradle will raise an error if the import-order violating source file exists. If you have any proposals or ideas, don't hesitate to give me a reply. I will create a regular issue of it and update the PR. Regards, Dongjin [^1]: In my case, I am using CheckStyle-IDEA: https://plugins.jetbrains.com/plugin/1065-checkstyle-idea [^2]: https://github.com/diffplug/spotless/tree/main/plugin-gradle On Sun, Oct 25, 2020 at 1:12 AM Matthias J. Sax <mj...@apache.org> wrote: > Hi, > > Personally I don't care about which order we choose, but having one > seems reasonable. > > My main ask is, can we document clearly, how tools like IntelliJ and > Eclipse need to be configured such that they obey the defined rules? It > would be very annoying of it would be required to disable "auto import" > and manually write import statements to obey the rules. > > -Matthias > > On 10/23/20 7:19 PM, John Roesler wrote: > > 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 > >>>>> > >>>> > >>>> > >>> > >> > > -- *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>*
