Skip to content

fix: spotless plugin activation issue #2704

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

fix: spotless plugin activation issue #2704

wants to merge 2 commits into from

Conversation

xstefank
Copy link
Collaborator

Fixes #2697

@openshift-ci openshift-ci bot requested review from csviri and metacosm February 25, 2025 16:36
@xstefank
Copy link
Collaborator Author

@csviri unforunately, I can't find a way to allow mvn spotless:apply from everywhere. Not it reqires mvn package so the directory-maven-plugin sets correctly the root directory for the config files. Is it ok like this or should we revert back to not allow building of inner modules? I think this is better.

EDIT: I see that we use spotless:check in GH actions so that would need to be reworked.

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 25, 2025
metacosm
metacosm reviewed Feb 26, 2025
View reviewed changes
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/Utils.java Outdated
};
}
}
static boolean getBooleanFromSystemPropsOrDefault(String propertyName,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, so I guess the switch issue with the new spotless version is not resolved, it's more that it wasn't getting applied anymore… We should probably revert to an older version then because that formatting of switch statements is a no-go.

@xstefank xstefank force-pushed the spotless-fix-2697 branch 2 times, most recently from 372f6ba to bca9e17 Compare February 26, 2025 09:44
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 26, 2025
xstefank
xstefank commented Feb 26, 2025
View reviewed changes
contributing/eclipse-google-style.xml
@@ -65,7 +65,7 @@
<setting id="org.eclipse.jdt.core.formatter.comment.format_line_comments" value="true"/>
<setting id="org.eclipse.jdt.core.formatter.insert_space_after_closing_angle_bracket_in_type_arguments" value="do not insert"/>
<setting id="org.eclipse.jdt.core.formatter.insert_space_after_comma_in_enum_declarations" value="insert"/>
<setting id="org.eclipse.jdt.core.formatter.join_wrapped_lines" value="false"/>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@csviri @metacosm this is the only change I had to do in order for spotless to not mess things up. But it unrolled a lot of lines.

Copy link
Collaborator

@csviri csviri Feb 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I would rather stay on an older version while is this fixed in spotless, or is it not expected to be fixed?
can we create an issue for spotless regarding this?

@xstefank
Copy link
Collaborator Author

I propose to move to this https://github.com/diffplug/spotless/blob/main/plugin-maven/README.md#google-java-format which would remove the need for a custom config file and should follow latest google styles - https://github.com/google/google-java-format/releases. If agreed, I can make a new PR with this move so we avoid back and forth formatting changes. I really don't like that we can't keep the same format options because spotless changed.

@csviri
Copy link
Collaborator

csviri commented Feb 26, 2025

I propose to move to this https://github.com/diffplug/spotless/blob/main/plugin-maven/README.md#google-java-format which would remove the need for a custom config file and should follow latest google styles - https://github.com/google/google-java-format/releases. If agreed, I can make a new PR with this move so we avoid back and forth formatting changes. I really don't like that we can't keep the same format options because spotless changed.

could you pls do a separate PR to see how that would look like?

@xstefank
Copy link
Collaborator Author

@csviri

google-java-format - #2706
palantir - #2707

@xstefank xstefank closed this Mar 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@metacosm metacosm metacosm left review comments

@csviri csviri Awaiting requested review from csviri

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Spotless plugin activation issue
4 participants
@xstefank @csviri @metacosm @openshift-merge-robot