Ivan,
Facts beat hand wavy assertions. You are of course wrong, there is a
pattern in the code, but I was also wrong. The code has many more files
with static imports first. I asked some AI to create me a script to check,
and it gave me:
import os
import re
def find_java_files_with_static_imports_first(directory):
java_files_with_issues = []
for root, _, files in os.walk(directory):
for file in files:
if file.endswith(".java"):
file_path = os.path.join(root, file)
with open(file_path, 'r') as f:
content = f.read()
static_imports = re.findall(r'^import static .+;$',
content, re.MULTILINE)
non_static_imports = re.findall(r'^import
(?!static).+;$', content, re.MULTILINE)
if static_imports and non_static_imports:
first_static_import =
content.find(static_imports[0])
first_non_static_import =
content.find(non_static_imports[0])
if first_static_import < first_non_static_import:
java_files_with_issues.append(file_path)
return java_files_with_issues
directory = '/Users/sodonnell/source/ozone2'
files_with_issues = find_java_files_with_static_imports_first(directory)
if files_with_issues:
print("Java files with static imports before non-static imports:")
for file in files_with_issues:
print(file)
else:
print("No issues found.")
Running that:
% python3 find.py | wc -l
220
Changing "if first_static_import < first_non_static_import:" to "if
first_static_import > first_non_static_import:"
% python3 find.py | wc -l
1552
So it is much more common to have the static imports first. I checked a few
of the output files by hand, and the output appears correct.
On Mon, Feb 10, 2025 at 12:27 PM Ivan Zlenko <ivan.zle...@gmail.com> wrote:
> 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
> > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
