Skip to content

Add checkstyle configuration based on Google Java style#34

Merged
chris-allan merged 4 commits intoglencoesoftware:masterfrom
sbesson:checkstyle
Sep 23, 2025
Merged

Add checkstyle configuration based on Google Java style#34
chris-allan merged 4 commits intoglencoesoftware:masterfrom
sbesson:checkstyle

Conversation

@sbesson
Copy link
Copy Markdown
Member

@sbesson sbesson commented Sep 9, 2025

Fixes #33

24ca578 adds the checkstyle plugin and the Google Java style configuration for Google Java style from https://checkstyle.org/google_style.html.
cacb96e, cd75d5c and 1a4a5e4 make amendments this configuration to:

All other commits update the omero-zarr-pixel-buffer implementation to comply with the style configuration. Most of these updates are fairly standard and should be non controversial (indentation, import ordering, camel-case style). There are a few styling rules that differ from other repositories e.g. the requirement to not have operators like || at the end of a line. Leaving reviewers to establish whether it would be preferable to relax the rules instead of fixing the code.

@sbesson sbesson requested review from chris-allan and kkoz September 9, 2025 07:57
Copy link
Copy Markdown
Member

@kkoz kkoz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple wording suggestions. Otherwise looks good

@sbesson sbesson requested a review from kkoz September 19, 2025 12:59
Copy link
Copy Markdown
Member

@chris-allan chris-allan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fully supportive of the checkstyle configuration. I think my only issue with this is the construction of the commits and how/if we attempt to correctly utilize git-blame-ignore-refs.

Do you have thoughts on that already, @sbesson?

@sbesson
Copy link
Copy Markdown
Member Author

sbesson commented Sep 23, 2025

Thanks @chris-allan, a .git-blame-ignore-refs should be added to the top-level of this repository to improve examination of code changes moving forward.

The the current commit organisation was made with reviewing in mind to simplify the reversal of individual rules if needed. If we all agree on the content, these could be consolidated into four commits and this branch updated (via force push) as follows:

  • one commit to add the original Google checkstyle configuration and the build.gradle updates
  • one commit to add our modifications (indentations)
  • one commit with all the changes to the Java code to comply with the checkstyle
  • one commit to exclude the previous reference via .git-blame-ignore-refs

If we'd rather not force-push, I would add all commits a5221d0 up to ff26161 to .git-blame-ignore-refs

@chris-allan
Copy link
Copy Markdown
Member

Let's go with the force push and see how things look.

@sbesson sbesson force-pushed the checkstyle branch 2 times, most recently from 3e39dba to 0a40420 Compare September 23, 2025 13:49
- fix package and import errors
- fix whitespace errors
- fix Javadoc errors
- fix variable name errors
- fix indentation errors
- fix curly braces errors
- fix errors by calling constants static final
- fix long line errors
- move operators to a newline
- fix maximum distance between variable declaration and usage
@sbesson
Copy link
Copy Markdown
Member Author

sbesson commented Sep 23, 2025

Took several force pushes but the diff between the previous history and the new history with squashed commits and the .git-blame-ignore-revs addition is reviewable at https://github.com/glencoesoftware/omero-zarr-pixel-buffer/compare/9e8d92a1bfc2ece12384d9626b096ec11afea944..f683540

@chris-allan chris-allan merged commit edc942f into glencoesoftware:master Sep 23, 2025
6 checks passed
@sbesson sbesson deleted the checkstyle branch September 23, 2025 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Coding conventions

3 participants