Stephen, Look, it was my first line of thought to come up with a general pattern for a checkstyle, so it will be easier to change and review. But after going through the whole Ozone code base and creating 33 branches with fixes I can safely say - there is no pattern.
Sincerely yours, Ivan Zlenko. On Mon, 10 Feb 2025 at 16:41, Stephen O'Donnell <sodonn...@cloudera.com.invalid> wrote: > >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 > > > > > > > > > > > > > > > > > > > > > > > > > > >
