spotless
spotless copied to clipboard
diffplug
Support Scalafix
The PR adds support for Scalafix which is widely used as well as Scalafmt. In addition to ScalaFixStep, I modified ScalaExtension to add a dependency on scala compilation as Scalafix needs compilation before formatting.
Hi @hnoson. Just wanted to give you some insight into the merge conflicts. We've maintained support back to Gradle 2.x, and we're finally breaking that and adopting all the latest Gradle features. I think your PR is mostly unaffected, except that pretty soon you could use Gradle 5.4+ as the baseline, which might be helpful. My guesstimate is that we'll have removed all deprecated code, and fully bumped the minimum to Gradle 5.4 in ~2 weeks. Part of that effort will be a big overhaul to the README, so there might be some conflicts there as well.
This is a great PR, and I hope to help it get merged in. I think it would be very doable to merge it in with the current Gradle 2.x baseline, but if that's daunting there will be more stability in a few weeks. Just FYI.
The brief upheaval in Spotless' core has ended. I fixed the merge conflicts and made some small edits to the PR to adjust for the new APIs in Spotless. All of the feedback above still stands.
One big upside is that when you started this PR, our minimum Gradle was 2.14. It is now 5.4 (and I'm open to going higher), so hopefully that will help in integrating with the scala plugin.
Thanks, and sorry for the delay. I believe I can resume working on this next week.
I am also interested in this feature, is it only merge conflicts with main that is necessary to get this PR through? I also noticed that the implementation is using reflection which may expose similar problems that the scalafmt plugin had until https://github.com/diffplug/spotless/pull/1283 was merged.
is it only merge conflicts with main that is necessary to get this PR through?
I don't think there's a huge amount of work left, but there is more than just merge conflicts.
The biggest problem is that AFAICT, scalafix needs the compiled code in order to operate. Which is fine, but we'll need to figure out a way to make spotlessScala depend on scalaCompile or something like that. Not a blocker, but a bit of work. The open code review comments above are all still valid.
the implementation is using reflection
Yeah, probably a good idea to switch to compile-time deps as you did for scalafmt.
This PR as it stands does a great job with the docs and scaffolding the implementation, and I'd love to merge a PR that takes this all the way.
The biggest problem is that AFAICT, scalafix needs the compiled code in order to operate. Which is fine, but we'll need to figure out a way to make spotlessScala depend on scalaCompile or something like that. Not a blocker, but a bit of work. The open code review comments above are all still valid.
There is also an additional problem which is that gradle doesn't have a standard way to handle scala. There is gradle-scala plugin however that has quite a few limitations (i.e. it doesn't really support cross compilation) and because of this there are plugins like https://github.com/ADTRAN/gradle-scala-multiversion-plugin.
This PR as it stands does a great job with the docs and scaffolding the implementation, and I'd love to merge a PR that takes this all the way.
I will have to take a deeper look into it but depending on how many things need to be changed I might make a PR myself and co-author @hnoson .
Sounds great. Don't feel a need to support every use-case, just supporting your own use-case in a mostly-future-proof way is plenty to get merged. People can extend to support their own use-cases as needed.