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

Apply java and kotlin source formatting #20

Merged
merged 2 commits into from
Jan 8, 2025

Conversation

hillcg-aws
Copy link
Collaborator

Enable "spotless" plugin and configure that to (1) reference the default "google-java-format style" for Java and (2) use "ktlint" for Kotlin; followed by a bulk reformat of source code (via the new "spotlessApply" gradle task).

Enable "spotless" plugin and configure that to (1) reference the default
"google-java-format style" for Java and (2) use "ktlint" for Kotlin;
followed by a bulk reformat of source code (via the new "spotlessApply"
gradle task).
Copy link
Member

@robin-aws robin-aws left a comment

Choose a reason for hiding this comment

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

Heavily in favor of enforcing A code style of some kind, and we can always tweak/change it later. Just one sticking point for me below.

@@ -3,125 +3,133 @@
*/
package org.dafny.gradle.plugin;

import static org.junit.jupiter.api.Assertions.*;
Copy link
Member

Choose a reason for hiding this comment

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

This is the only point I disagree with: I don't like star imports because new classes showing up in new versions of other dependencies can break resolution of your code, and in my experience IDEs make managing imports easy enough these days that the convenience of star imports is negligable.

Is the formatter requiring this, or just allowing it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Interestingly it's not needed at all (vscode shows it as a warning) so can be removed. I'll add another commit to take it out.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To answer your question though, it's allowing it, not mandating it.

Copy link
Member

@robin-aws robin-aws left a comment

Choose a reason for hiding this comment

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

🚀

@robin-aws robin-aws merged commit 2607255 into dafny-lang:main Jan 8, 2025
1 check passed
@hillcg-aws hillcg-aws deleted the spotless branch January 8, 2025 19: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