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

Migrate from Checkstyle to Spotless #3874

Merged
merged 5 commits into from
Dec 18, 2023
Merged

Conversation

Scott-Guest
Copy link
Contributor

@Scott-Guest Scott-Guest commented Dec 12, 2023

This PR migrates all of our formatting checks from the Checkstyle plugin to the Spotless plugin.

As a result, mvn package (or mvn spotless:apply or mvn process-sources) automatically fixes any formatting violations, e.g., inserting copyright headers for you.

The diff is large, but the only non-automated code changes are in pom.xml and src/main/config/checkstyle-*.xml

All the behavior is the exact same except:

  • Previously, copyright headers could match the regex Copyright \(c\) (Runtime Verification, Inc|K Team). All Rights Reserved., but now we only accept the Runtime Verification, Inc version due to limitations of Spotless. All existing headers were converted automatically
    • If there is an issue with this legally, I believe there is a mechanism to avoid changing the copyright of any existing files, but all new files would still require Runtime Verification, Inc
  • We more broadly enforce spaces over tabs, unix line endings, newlines at the end of each file, and no trailing whitespace. Previously, this was only enforced for K and Java files.

@Scott-Guest Scott-Guest force-pushed the checkstyle-to-spotless branch from e1954e2 to f1b3be8 Compare December 12, 2023 22:21
@Scott-Guest Scott-Guest self-assigned this Dec 13, 2023
@Scott-Guest Scott-Guest marked this pull request as ready for review December 13, 2023 15:26
Copy link
Contributor

@radumereuta radumereuta left a comment

Choose a reason for hiding this comment

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

lgtm now, although very large.
Maybe someone else can have a look. If not, you can merge in a few days.

Copy link
Contributor

@Baltoli Baltoli left a comment

Choose a reason for hiding this comment

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

Looks good to me also (once my laptop had loaded the diff page!)

@rv-jenkins rv-jenkins merged commit b5c6368 into develop Dec 18, 2023
8 checks passed
@rv-jenkins rv-jenkins deleted the checkstyle-to-spotless branch December 18, 2023 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants