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

Google Java format #115

Merged
merged 8 commits into from
Jun 11, 2024
Merged

Google Java format #115

merged 8 commits into from
Jun 11, 2024

Conversation

springmeyer
Copy link
Collaborator

We have prettier/ESLint for JS. This introduces consistent formatting tools for Java.

I don't personally have opinions about what formatter to use. Google's just seemed like a reasonable first place to start.

This uses https://github.com/diffplug/spotless/tree/main/plugin-gradle as a simple and efficient way to apply & enforce the Java format.

@nyurik
Copy link
Member

nyurik commented Jun 10, 2024

i'm all for the stable code formatting (thx!). My only concern is if we have any significant code change PRs, merging them would be a bit more difficult due to merge conflicts. Are there any PRs that should be merged first?

nyurik
nyurik approved these changes Jun 10, 2024
@springmeyer
Copy link
Collaborator Author

My only concern is if we have any significant code change PRs, merging them would be a bit more difficult due to merge conflicts. Are there any PRs that should be merged first?

Yes, I plan to hold on this until it's 100% certain everything in-flight is merged (will confirm with folks in slack). But, wanted to get this going for initial review. Will mark draft for now to indicate I don't plan to merge yet.

@springmeyer springmeyer marked this pull request as draft June 10, 2024 04:52
@springmeyer
Copy link
Collaborator Author

Noting that this PR intended to check 2 things:

    1. Confirming that the CI jobs for java failed, as intended, when adding a format check to the Gradle build + running ./gradlew build on CI ✅
    1. Confirming that the CI jobs passed for java when applying the java formatting locally and then pushing the results ✅

Now that 2 is confirmed, I'll revert 2 in order to make this PR mergable without merge conflicts against other in-flight PRs. Then we can circle back separately to decide when to apply the formatting overall. Sound good?

@nyurik
Copy link
Member

nyurik commented Jun 10, 2024

Sounds good. Do you want to make #116 change (file paths in org/maplibre/mlt style) here too?

@springmeyer springmeyer marked this pull request as ready for review June 10, 2024 20:28
@springmeyer
Copy link
Collaborator Author

Do you want to make #116 change (file paths in org/maplibre/mlt style) here too?

That feels like it could break in-flight PRs. So I'm thinking to merge this without additional/any breaking changes. Then followup with a formatting PR that changes the code in src/ and then yes, adding in #116 would be great.

@nyurik
Copy link
Member

nyurik commented Jun 11, 2024

hope we can merge this soonish (and run the global reformat)

@springmeyer springmeyer merged commit 62a065d into maplibre:main Jun 11, 2024
6 checks passed
@springmeyer springmeyer deleted the dane/java-lint branch June 11, 2024 21:47
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