You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Dongjin Lee <do...@apache.org> on 2020/10/15 05:36:08 UTC
[DISCUSS] Checkstyle Import Order
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>*
Re: [DISCUSS] Checkstyle Import Order
Posted by Dongjin Lee <do...@apache.org>.
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 <gw...@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 <do...@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>*
Re: [DISCUSS] Checkstyle Import Order
Posted by "Matthias J. Sax" <mj...@apache.org>.
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 <gw...@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 <do...@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
>>>>>
>>>>
>>>>
>>>
>>
Re: [DISCUSS] Checkstyle Import Order
Posted by John Roesler <vv...@apache.org>.
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 <gw...@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 <do...@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
> > >>
> > >
> > >
> >
>
Re: [DISCUSS] Checkstyle Import Order
Posted by Sophie Blee-Goldman <so...@confluent.io>.
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 <gw...@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 <do...@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
> >>
> >
> >
>
Re: [DISCUSS] Checkstyle Import Order
Posted by Bruno Cadonna <br...@confluent.io>.
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 <gw...@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 <do...@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
>>
>
>
Re: [DISCUSS] Checkstyle Import Order
Posted by Dongjin Lee <do...@apache.org>.
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 <gw...@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 <do...@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>*
Re: [DISCUSS] Checkstyle Import Order
Posted by Gwen Shapira <gw...@confluent.io>.
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 <do...@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
Re: [DISCUSS] Checkstyle Import Order
Posted by Brandon Brown <br...@bbrownsound.com>.
I like option 2.
Brandon Brown
> On Oct 15, 2020, at 1:36 AM, Dongjin Lee <do...@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>*
Re: [DISCUSS] Checkstyle Import Order
Posted by Bruno Cadonna <br...@confluent.io>.
Hi Dongjin,
Thank you for bringing this up.
I like options 2 and 3.
Best,
Bruno
On 15.10.20 07:36, Dongjin Lee 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
>