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

Add support for scalafmt >= v0.7.0-RC1 #124

Merged
merged 1 commit into from
Jul 11, 2017
Merged

Add support for scalafmt >= v0.7.0-RC1 #124

merged 1 commit into from
Jul 11, 2017

Conversation

alexvetter
Copy link
Contributor

In scalafmt v0.7.0-RC1 the configuration API has been changed a little bit. This PR adds a pourman's switch (try... catch...) to support both older and new versions.

  • Add support for scalafmt >= v0.7.0-RC1
  • Still supports scalafmt <= v0.6.8
  • Make v1.1.0 the default scalafmt version

@jbduncan
Copy link
Member

Hi @alexvetter, thanks for the PR!

Would you kindly run gradlew spotlessApply and squash & force push a new commit into the existing one, so that Travis and Spotless don't complain about the formatting.

@alexvetter
Copy link
Contributor Author

alexvetter commented Jul 11, 2017

Hi @jbduncan! Sorry, my bad. It should have been obvious to run spotlessApply before submitting a PR. I hope it works now.

@jbduncan
Copy link
Member

No worries! Thanks for doing it so quickly. :)


Object configured = fromHocon.invoke(null, configStr, fromHoconEmptyPath);
either = invokeNoArg(configured, "toEither");
} catch (Exception e) {
Copy link
Member

Choose a reason for hiding this comment

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

Hmm... is it possible to reduce the scope from Exception to a specific subclass?

@alexvetter
Copy link
Contributor Author

  • Catch is now more specific (NoSuchMethodException).
  • Added a comment to the catch case.
  • Fixed the missed test.
  • Travis now approves ✅

@jbduncan
Copy link
Member

Thanks @alexvetter!

Unless @nedtwigg has anything else to say, this would probably be safe to merge in.

@nedtwigg, do you have any thoughts?

@nedtwigg nedtwigg merged commit 519e442 into diffplug:master Jul 11, 2017
@nedtwigg
Copy link
Member

LGTM, thanks for the contribution! I'll be turning the release crank shortly.

@nedtwigg
Copy link
Member

Released in 3.4.1

@alexvetter
Copy link
Contributor Author

Thanks, that was fast!

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.

3 participants