Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: change checkstyle to google code format, plus adding spotless #1121

Conversation

aepfli
Copy link
Member

@aepfli aepfli commented Jan 1, 2025

We regularly fight with code styles in pull requests, which is annoying to both the contributors and the maintainers.

There are tools out there, like Spotless, that can automate those processes. We could define our code style, but there is already one widely used code style from Google (Google Java format).

Existing plugins help us follow this code style within IntelliJ and Visual Studio Code. Switching to this definition can improve the overall experience.

This supersedes the pull request with .editorconfig

notes

  • first commit contains the changes to the tooling
  • second commit contains the applied formatting changes

Disclaimer

This pull request should be used to discuss this; most people do not have significant opinions regarding code styles as long as there are tools to enforce them. Now we have a maven target, spotless:apply, which will inform you about violations and fix them.

We are regularly fighting with codestyles in pullrequests, which is
annoying to the contributors, as well as the maintainers.

There are tools out there, like spotless, which can automate those
processes. We could define an own codestyle, but in fact there is
already one widely used codestyle from google (google java format).

There are plugins to help us within intellij and vscode to follow
this code style. Hence the overall experience can improve by
switching to this defitinion.

Signed-off-by: Simon Schrottner <simon.schrottner@dynatrace.com>
Copy link
Member

@justinabrahms justinabrahms left a comment

Choose a reason for hiding this comment

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

Seems.. fine? No question from me that style should be enforced by robots.

From the slack thread, I expected the rule changes to be more significant. Is the real problem here "The checkstyle that we had before is under-specified and we need to use more modules; Also... add spotless plugin"?

pom.xml Show resolved Hide resolved
@aepfli
Copy link
Member Author

aepfli commented Jan 1, 2025

Seems.. fine? No question from me that style should be enforced by robots.

From the slack thread, I expected the rule changes to be more significant. Is the real problem here "The checkstyle that we had before is under-specified and we need to use more modules; Also... add spotless plugin"?

The check style changes are more related to be fact that i wanted to use spotless to ease the pain. But our check style configuration was not compatible with the default settings.

As I favor simplicity over custom specification, I thought it is easier to migrate to google check style and a adapt the full java code Format configuration, rather than making spotless work with our current check style config.

Copy link
Member

@justinabrahms justinabrahms left a comment

Choose a reason for hiding this comment

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

"Make config work well w/ the auto enforcer" works for me as a rationale. Thanks so much!

@aepfli aepfli force-pushed the build/add_spotless_for_autoformatting branch 2 times, most recently from 61112ba to fefce1b Compare January 2, 2025 08:52
@guidobrei
Copy link
Member

Thanks so much for adding this!
Should we also document this in the Contribution guide?

@aepfli aepfli force-pushed the build/add_spotless_for_autoformatting branch from fefce1b to 871c825 Compare January 2, 2025 10:12
Signed-off-by: Simon Schrottner <simon.schrottner@dynatrace.com>
@aepfli aepfli force-pushed the build/add_spotless_for_autoformatting branch from 871c825 to b820457 Compare January 2, 2025 10:13
@aepfli
Copy link
Member Author

aepfli commented Jan 2, 2025

open-feature/java-sdk#1264 - sibling pr for java-sdk

Signed-off-by: Simon Schrottner <simon.schrottner@dynatrace.com>
@aepfli
Copy link
Member Author

aepfli commented Jan 2, 2025

Thanks so much for adding this! Should we also document this in the Contribution guide?

added a section, with a detailed explanation

Copy link
Member

@toddbaert toddbaert left a comment

Choose a reason for hiding this comment

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

As a vscode user, thanks so much! ❤️

Signed-off-by: Simon Schrottner <simon.schrottner@dynatrace.com>
@aepfli aepfli merged commit e612a65 into open-feature:main Jan 3, 2025
4 checks passed
@aepfli aepfli deleted the build/add_spotless_for_autoformatting branch January 3, 2025 17:35
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.