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 Windows bug workaround now only runs for affected version (1.1). #73

Merged
merged 2 commits into from
Jan 20, 2017

Conversation

jbduncan
Copy link
Member

No description provided.

@jbduncan
Copy link
Member Author

Related to #65.

@nedtwigg
Copy link
Member

LGTM!

@nedtwigg
Copy link
Member

Oh, except I didn't realize that it didn't bump the default version to 1.2. I think it should definitely use 1.2.

@jbduncan
Copy link
Member Author

Oh, except I didn't realize that it didn't bump the default version to 1.2. I think it should definitely use 1.2.

Okey dokey! I've pushed a new commit fixing this at 1250f5a.

@jbduncan
Copy link
Member Author

Notes for self:

  1. Update RemoveUnusedImports.java to use google-java-format 1.2.
  2. Find a way to test that fixWindowsBug is only run if google-java-format version is 1.1. @nedtwigg Do you have any thoughts on how I could do this?

@nedtwigg
Copy link
Member

  1. I think that's already done:
    () -> new GoogleJavaFormatStep.State(NAME, GoogleJavaFormatStep.defaultVersion(), provisioner),
  2. Perfect world would have a test, I don't see a good way to do it. I think we should merge, and then test 3.1.0-SNAPSHOT on junit. If removeUnusedImports now works for them, that's good enough for me. Feel free to merge at your convenience / satisfaction :)

@jbduncan
Copy link
Member Author

  1. Oh yes, you're right! Thanks for pointing that out for me.
  2. Okay, in that case I agree on merging this as-is. Doing that now. :)

@jbduncan
Copy link
Member Author

@nedtwigg, how do you prefer PRs to be merged in for Spotless:

  1. Merge commit,
  2. Squash and merge commit,
  3. Rebase and commit on top of master?

@nedtwigg
Copy link
Member

Merge commit. Easy to revert if there's a problem, keeps history intact.

@jbduncan jbduncan merged commit c8bbac4 into master Jan 20, 2017
@jbduncan
Copy link
Member Author

Done. :)

@jbduncan
Copy link
Member Author

Hi @nedtwigg, I've just tested the latest spotless-plugin-gradle snapshot against JUnit 5 snapshot with the removeUnusedImports() option enabled.

AFAICT, it works beautifully! So we should consider releasing 3.1 soon. :)

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