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

Code formatting via Spotless with google java format #653

Merged
merged 3 commits into from
Sep 1, 2021

Conversation

gotson
Copy link
Collaborator

@gotson gotson commented Aug 31, 2021

as discussed here: #651

@gotson
Copy link
Collaborator Author

gotson commented Aug 31, 2021

@xerial by binding Spotless to the compile phase, the CI running on Ubuntu JDK 8 fails. What do you want to do about it ?

Options I see:

  1. drop JDK 8 from CI
  2. bind spotless to the verify phase, and run a separate CI job to check formatting

The problem with option 2 is that people could very much commit and push code without running spotless, so we would catch the formatting at CI only, making the feedback loop longer.

@gotson
Copy link
Collaborator Author

gotson commented Aug 31, 2021

One more solution would be to compile in CI using jdk 11, but run the ci job with jdk8. That would probably mean splitting the job in 2, and store artifacts temporarily.

@xerial
Copy link
Owner

xerial commented Aug 31, 2021

I prefer

  1. bind spotless to the verify phase, and run a separate CI job to check formatting

@gotson
Copy link
Collaborator Author

gotson commented Sep 1, 2021

I prefer

  1. bind spotless to the verify phase, and run a separate CI job to check formatting

I changed it, please have a look when you have time 🙏🏻

@xerial xerial merged commit 3d04d7d into xerial:master Sep 1, 2021
@xerial
Copy link
Owner

xerial commented Sep 1, 2021

LGTM. Thanks for fixing the long standing issue :)

@gotson gotson deleted the spotless branch September 1, 2021 23:44
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.

2 participants