>From my observation it is 50/50 for Ozone codebase in terms of static/non-static imports ordering.
I happened to have 9 random files open in Intellij, and checked them. All had static imports last, and based on my experience on the project over the last 5 years, statics are almost always last. Possibly that is an Intellij default and that is why. So I would encourage actually checking this 50:50 assertion and picking the one that is actually correct, rather than creating unnecessary change in the code base. These kind of changes make backports to older branches more difficult, as they will nearly always cause conflicts. On Mon, Feb 10, 2025 at 10:20 AM Ivan Zlenko <ivan.zle...@gmail.com> wrote: > Sammi, > > From my observation it is 50/50 for Ozone codebase in terms of > static/non-static imports ordering. > As I've said before I've already finished with Ozone checkstyle for all > modules and now waiting on the first to be merged so I can create all > follow up pull requests. > > Sincerely yours, > Ivan Zlenko. > > On Mon, 10 Feb 2025 at 15:15, Sammi Chen <sammic...@apache.org> wrote: > > > Ivan, > > > > Thank you for the explanation. I adjusted the intellij configuration > > accordingly. > > > > There is one observation. I'm using "IntelliJ IDEA 2022.1.3" which seems > > by default to put static imports after normal imports, which is the > > opposite order of the new proposal. > > > > If we use the new order, then the majority of source code files need to > be > > updated, which will be a lot of effort. > > > > Shall we keep using the current non-static first then static order? As > long > > as we follow the same order, we can achieve a consistent style too. > > > > Bests, > > Sammi > > > > > > > > > > On Mon, 10 Feb 2025 at 17:46, Ivan Zlenko <ivan.zle...@gmail.com> wrote: > > > > > Hi, Sammi. > > > > > > > BTW, I 'm not sure if Intellij has any way to help enforce these > rules > > > so it would be easy to follow them. > > > > > > It will be straightforward to set up these rules in IntelliJ Idea. > Right > > > now I have changes for all modules in Ozone to make them compliant to > the > > > new checkstyle and all changes were done in an almost automatic way > using > > > the "Optimize imports on the fly" option. > > > > > > The only thing you need to do is the following: > > > 1. Set class count for * import into something like 999, to prevent any > > > attempt to add * imports. > > > 2. Set layout as following: > > > import static all other imports > > > <blank line> > > > import all other imports. > > > > > > > "customImportOrderRules" and "tokens", can we have a little > explanation > > > of the rule here? > > > > > > The rules are pretty simple: sort all imports in alphabetical order, > > > separate in 2 groups starting with static imports and ending with > common > > > imports, groups separated by whitespace. > > > > > > Sincerely yours, > > > Ivan Zlenko. > > > > > > > > > On Mon, 10 Feb 2025 at 14:34, Sammi Chen <sammic...@apache.org> wrote: > > > > > > > Thanks Attila and Ivan working on this improvement. > > > > > > > > IIUC, the major enforced rules of imports are > > > > > > > > <module name="CustomImportOrder"> > > > > > <property name="sortImportsInGroupAlphabetically" > > > > > value="true"/> > > > > > <property name="separateLineBetweenGroups" > value="true"/> > > > > > <property name="customImportOrderRules" > > > > > value="STATIC###THIRD_PARTY_PACKAGE"/> > > > > > <property name="tokens" value="IMPORT, STATIC_IMPORT, > > > > > PACKAGE_DEF"/> > > > > > </module> > > > > > > > > > > > > "sortImportsInGroupAlphabetically" and "separateLineBetweenGroups" > look > > > > straightforward. > > > > "customImportOrderRules" and "tokens", can we have a little > explanation > > > of > > > > the rule here? > > > > > > > > BTW, I 'm not sure if Intellij has any way to help enforce these > rules > > so > > > > it would be easy to follow them. > > > > > > > > Bests, > > > > Sammi > > > > > > > > On Mon, 10 Feb 2025 at 17:02, Attila Doroszlai < > adorosz...@apache.org> > > > > wrote: > > > > > > > > > Hi Ozone developers, > > > > > > > > > > Ivan Zlenko has started working on standardizing license header and > > > > > the order of imports. > > > > > > > > > > The first PR [1] adds the sample header [2], as well as checkstyle > > > > > rules for these two items. Rules will be initially disabled in all > > > > > submodules. The plan is to update submodules gradually, one at a > > > > > time, fixing existing files and enabling the rules. > > > > > > > > > > For new files please start using the standard header [2] and import > > > > > order (shown in the PR description), even if the rule is still > > > > > disabled in the specific module. This will help avoid accidental > > > > > violations if PRs are merged concurrently. > > > > > > > > > > We will help resolve conflicts in open PRs. Please avoid merging > PRs > > > > > with old CI results. > > > > > > > > > > thanks, > > > > > Attila > > > > > > > > > > [1] https://github.com/apache/ozone/pull/7836 > > > > > [2] > > > > > > > > > > > > > > > https://github.com/apache/ozone/blob/966145d5e869263d53c516d36a0c02e058fd24b9/hadoop-hdds/dev-support/checkstyle/license.header > > > > > > > > > > > --------------------------------------------------------------------- > > > > > To unsubscribe, e-mail: dev-unsubscr...@ozone.apache.org > > > > > For additional commands, e-mail: dev-h...@ozone.apache.org > > > > > > > > > > > > > > > > > > > >
