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

[Spike] Try out Spotless; outcome: Do include Spotless in the project #489

Closed
jwlibby opened this issue May 22, 2024 · 4 comments
Closed
Labels
spike Explore options and questions

Comments

@jwlibby
Copy link
Collaborator

jwlibby commented May 22, 2024

Try out Spotless and see what you think. When evaluating refer to the questions in #462 and upon completion add your pros and cons for this plugin as a comment to the aforementioned story. Plugin link: Spotless.

Considerations for implementation cards:

  • Gradle: easy
  • Maven: lots of challenges, dependencies: Plexus, SLF4J, Kotlin, Eclipse; the plugin dependency wants Sonatype rather than Maven Central also easy. The challenges and dependencies issues mentioned were because I had incorrectly added the plugin as a dependency, not as a plugin

Related: #534 for epic on dependency management.

@jwlibby jwlibby added the spike Explore options and questions label May 22, 2024
@jwlibby jwlibby moved this to Backlog in @binkley's Modern Build May 22, 2024
@jwlibby jwlibby removed their assignment May 22, 2024
@jwlibby jwlibby moved this from Backlog to Ready in @binkley's Modern Build May 22, 2024
@binkley binkley moved this from Ready to In progress in @binkley's Modern Build Jun 4, 2024
@binkley binkley self-assigned this Jun 4, 2024
@binkley binkley moved this from In progress to Ready in @binkley's Modern Build Jun 4, 2024
@binkley binkley removed their assignment Jun 4, 2024
@jwlibby jwlibby self-assigned this Jun 5, 2024
@jwlibby jwlibby moved this from Ready to In progress in @binkley's Modern Build Jun 5, 2024
@binkley binkley moved this from In progress to Ready in @binkley's Modern Build Jun 10, 2024
@binkley binkley moved this from Ready to In progress in @binkley's Modern Build Jun 10, 2024
@jwlibby
Copy link
Collaborator Author

jwlibby commented Jun 10, 2024

  • Does the plugin work with both Gradle and Maven? Yes (also works with SBT)
  • How does this mesh with Checkstyle? Or can the plugin replace Checkstyle? It can replace it, it is the same plus more functionality
  • Can the plugin automatically reformat the code as part of the local build process? yes
  • If code is reformatted before pushing, how will the programmer be aware of this? we would manually set a step to log that such happened to the console
  • Does the plugin have good backward/forward compatibility and adaptability to different versions of the language, its dependencies and the runtime environment (jvm)? for java I think so since java itself is backwards compatible. Plus there is an attribute to format according to a particular java version
  • Is the plugin configurable in its rules to meet my team standards? compare to what is in checkstyle configuration
  • Should the book recommend a "one true" formatting style, provide a list of common styles, or leave that up to each team? No
  • Book tries to be agnostic on most things. An existing example is the checkstyle settings in config/checkstyle/*. not sure ab this question

@jwlibby
Copy link
Collaborator Author

jwlibby commented Jun 10, 2024

spike can be found in spotless-spike branch

@binkley
Copy link
Owner

binkley commented Jun 11, 2024

Note: the spotless-spike branch has a mistake in using Spotless as a dependency rather than a plugin.

@jwlibby jwlibby moved this from In progress to In review in @binkley's Modern Build Jun 11, 2024
@binkley
Copy link
Owner

binkley commented Jun 12, 2024

@jwlibby The only feedback I have on the writing is this:

* [Spotless](https://github.com/diffplug/spotless) — Focus on Google style guides for Java.

I'm unsure what style to suggest here. For example, I noticed the Google style includes the "AOSP" variant that uses 4-space indentation rather than Google's odd 2-space indentation. Palantir is also an interesting choice.

So perhaps soften the language to not offer a recommendation on style choice.

@jwlibby jwlibby moved this from In review to Done in @binkley's Modern Build Jun 12, 2024
@binkley binkley closed this as completed Jun 12, 2024
@binkley binkley changed the title Try out Spotless Try out Spotless; outcome: Do include Spotless in the project Jun 19, 2024
@binkley binkley changed the title Try out Spotless; outcome: Do include Spotless in the project [Spike] Try out Spotless; outcome: Do include Spotless in the project Jun 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spike Explore options and questions
Projects
Status: Done
Development

No branches or pull requests

2 participants