Skip to content
This repository has been archived by the owner on Jul 14, 2023. It is now read-only.

Use new IntelliJ import order #143

Merged
merged 2 commits into from
Dec 28, 2020
Merged

Conversation

sideeffffect
Copy link
Contributor

IntelliJ IDEA has recently changed the default order of import blocks. Would you @liancheng consider changing to the new scheme?

Since IDEA is the most used Scala IDE, it would make life much simpler for the majority of scalafix-organize-imports users.

@liancheng
Copy link
Owner

liancheng commented Dec 16, 2020

Hey @sideeffffect, thanks for the PR. However, this would be a breaking behavior change and may break a lot of projects' CI builds. In fact, I'm surprised that IntelliJ made this change due to similar reasons. Especially, I don't see the new order is so much better than the existing one and worth a breaking behavior change.

BTW, where can I find the announcement? My IntelliJ IDEA 2020.3 still uses the old order.

@sideeffffect
Copy link
Contributor Author

https://blog.jetbrains.com/scala/2020/11/26/enhanced-package-prefixes/

The java* and scala* imports now go together

@sideeffffect
Copy link
Contributor Author

@liancheng you are the scalafix-organize-imports author so you know what's best for it and I 100 % respect that. I also understand that changing defaults could (will?) cause issues for the current users.

I just know that, from my own experience and seeing others use it, that being different from IDEA (which is what most people use) is causing grief too. You either have to customize scalafix-organize-imports config (but first you have to figure out what that config should be), or you have to live with IDEA fighting with scalafix-organize-imports. Then, very often happens, that you open a PR to see it fail in CI, because you used IDEA's Ctrl+O to sort the imports, so then you have to run Scalafix again, push a new commit and wait for CI again -- this can feel frustrating.

I was thinking that IDEA changing it's default scheme could be a good opportunity for scalafix-organize-imports to jump on the bandwagon. But there will be other opportunities in the future, like the 0.5.0 release, or 1.0.0. It could be the case, that the sooner we bite the bullet, the less it will hurt the whole community.

(@jastice are you guys planing any further changes to the order of Scala imports?)

Ultimately, this is your decision to make, of course 👍

@liancheng
Copy link
Owner

Hey @sideeffffect, sorry for the late reply, and thanks as always for your input! I'm convinced that lowering the migration cost of IntelliJ users is more important, especially when this project is still young and far from 1.0.

Changing the default groups also requires updating the doc and test cases, I can work on that.

@codecov
Copy link

codecov bot commented Dec 27, 2020

Codecov Report

Merging #143 (92b9857) into master (b263031) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #143   +/-   ##
=======================================
  Coverage   90.90%   90.90%           
=======================================
  Files           4        4           
  Lines         242      242           
  Branches       12       12           
=======================================
  Hits          220      220           
  Misses         22       22           
Impacted Files Coverage Δ
...les/src/main/scala/fix/OrganizeImportsConfig.scala 100.00% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b263031...92b9857. Read the comment docs.

@sideeffffect
Copy link
Contributor Author

I've updated the documentation and fixed the tests.
Please let me know, if you think there's more to be done.

@liancheng
Copy link
Owner

Thank you, merging!

@liancheng liancheng merged commit 869e427 into liancheng:master Dec 28, 2020
@sideeffffect sideeffffect deleted the patch-1 branch December 28, 2020 03:04
@liancheng liancheng added this to the v0.5.0 milestone Jan 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants