Skip to content

Customize artifact of google java format#944

Merged
nedtwigg merged 1 commit into
diffplug:mainfrom
figroc:feat-gjf-artifact
Sep 27, 2021
Merged

Customize artifact of google java format#944
nedtwigg merged 1 commit into
diffplug:mainfrom
figroc:feat-gjf-artifact

Conversation

@figroc

@figroc figroc commented Sep 22, 2021

Copy link
Copy Markdown
Contributor

Add artifact option to GoogleJavaFormatStep.

The Google Java Format is not friendly for lambda. I've managed a customized artifact to improve it. This PR allows user to integrate home-brewed GJF with spotless.

@figroc figroc changed the title feat: customize artifact for google java format Customize artifact of google java format Sep 22, 2021

@nedtwigg nedtwigg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Happy to merge this, needs:

  • misc changes which I marked up inline
  • a teensy callout within the README for plugin-gradle and plugin-maven

Comment thread plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JavaExtension.java Outdated
Comment thread plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JavaExtension.java Outdated
Comment thread lib/src/main/java/com/diffplug/spotless/java/GoogleJavaFormatStep.java Outdated
@figroc

figroc commented Sep 22, 2021

Copy link
Copy Markdown
Contributor Author

@nedtwigg Thank you for your insight.

Should I keep the MAVEN_COORDINATE naming?

@nedtwigg

Copy link
Copy Markdown
Member

Yes please :)

@figroc

figroc commented Sep 22, 2021

Copy link
Copy Markdown
Contributor Author

@nedtwigg Would you take another look? groupArtifact is used instead of mavenCoordinate.

@nedtwigg nedtwigg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just a few nits and I think this can get merged. You need it, and that's good enough for it to get merged.

What I don't like about it is that I think this feature is very niche, and not many people will need it. In general, I think it's good to gate stuff like that behind a custom name to indicate "you probably don't need this if you're just exploring". But maybe I'm wrong, and there are more forked GJF versions out there than I realize. So I'm okay with the naming and API as-is.

I'd like to let this cool off for 48 hrs to see if any other maintainers want to weigh-in, then I'll merge and release.

Comment thread plugin-maven/README.md
Comment thread plugin-maven/README.md Outdated
Comment thread lib/src/main/java/com/diffplug/spotless/java/GoogleJavaFormatStep.java Outdated
@nedtwigg

Copy link
Copy Markdown
Member

This looks great, all my concerns are satisfied. I'll merge and release sometime Thursday unless somebody objects between now and then.

@figroc

figroc commented Sep 24, 2021

Copy link
Copy Markdown
Contributor Author

@nedtwigg I forgot to add test for the feature. Please wait for my update.

@figroc

figroc commented Sep 24, 2021

Copy link
Copy Markdown
Contributor Author

@nedtwigg done adding unit tests

@figroc figroc requested a review from nedtwigg September 24, 2021 11:27
@figroc

figroc commented Sep 26, 2021

Copy link
Copy Markdown
Contributor Author

@nedtwigg I've rebased the PR. It seems there is some problem with ci/circleci: assemble_testClasses cloning the repo. What should I do?

@nedtwigg

Copy link
Copy Markdown
Member

Sorry, I'm stalled on #950. You've gone your part, I'll do mine asap.

@nedtwigg

Copy link
Copy Markdown
Member

Please merge the latest origin/main into this PR. That should fix CI, at which point I can merge and release.

update change log

add new ctor of State instead of changing existed ones

change artifact to groupArtifact

check groupArtifact content

add tests for custom group artifact
@nedtwigg nedtwigg merged commit 4e30602 into diffplug:main Sep 27, 2021
@nedtwigg

Copy link
Copy Markdown
Member

Published in plugin-gradle 5.15.2 and plugin-maven 2.14.0.

@nedtwigg

Copy link
Copy Markdown
Member

FYI, palantir-java-format is a lambda-friendly fork of GJF which shipped in Spotless plugin-gradle 6.2.0 and plugin-maven 2.20.0.

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