opwvhk commented on code in PR #3150: URL: https://github.com/apache/avro/pull/3150#discussion_r1773516333
########## lang/java/compiler/src/test/java/org/apache/avro/compiler/specific/TestSpecificCompiler.java: ########## @@ -17,14 +17,19 @@ */ package org.apache.avro.compiler.specific; -import static org.hamcrest.CoreMatchers.equalTo; Review Comment: It took a while, but I've delved into this a bit: most other source files use a "java first, then others and static last" approach. However, many tests use a "static first, then java, then others" approach. Also, many files contain missorted imports. It's probably best to simply stick to a single, preferably common, standard. For that, there are 4 major choices: **CheckStyle**, **Google**, **Eclipse**, and **JetBrains**, with the latter two modified to not allow * imports (this mostly affects the JetBrains style). Using these code styles (and two custom styles inferred from the code) yield the following number of changed files: | Style | .editorconfig approximation | Files changed | |------------|------------------------------------------------------|---------------| | Custom 1 | `java.**,javax.**,\|,*,\|,$*` | 389 (51%) | | Eclipse | `$*,\|,java.**,\|,javax.**,\|,org.**,\|,com.**,\|,*` | 391 (51%) | | JetBrains | `*,\|,java.**,javax.**,\|,$*` | 454 (60%) | | Custom 2 | `$*,\|,*,\|,java.**,javax.**` | 516 (68%) | | Google | `$*,\|,*` | 548 (72%) | | CheckStyle | `*,$*` | 554 (73%) | As you can see from the number (and percentage) of changed files, if the codebase ever followed an import code style, this was a very long time ago. Taking a different approach, finding static imports and JDK (java/javax) imports among all and 1st imports, does describe a pattern: * there are 233 files with static imports, of which 230 (99%) sort them before non-static imports * there are 635 files with JDK imports, of which 109 (17%) sort them before other (non-static) imports Using these two metrics, we should: * sort static imports before non-static imports * sort java/javax imports after other imports This yields the code style labelled "Custom 2" in the table above. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
