-
Notifications
You must be signed in to change notification settings - Fork 25.3k
Order imports when reformatting #74059
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
Conversation
Pinging @elastic/es-delivery (Team:Delivery) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW I'm a bit on the fence on this one unless we can have this configured on import automatically. A lot of this manual IDE config stuff is missed by developers. If someone doesn't do this they are going to be in for a very frustrating time battling between IDE rules and spotless.
I'll quote David here:
yes sorting differently from IntelliJ would definitely be worse than today’s lack of sorting
We need to factor in that if this isn't configured automatically a non-negligible number of folks are going to hit this issue. I think having manual setup for things that are "optional" is fine, but I consider having import ordering rules match between the IDE and build be pretty much mandatory given that basically everyone uses this IDE feature.
I think we can commit some stuff to |
So yeah, let's set up the IDE import config the way we want, export it to a file in |
@mark-vieira I tried your suggestion but struggled to get it to work. I asked in the IntelliJ forums and someone suggested using a Settings Repository. Not mad keen on introducing something new - what do you think? |
Yeah, that is not gonna be a good solution given that code style is a project setting and settings repositories are for making all your IDE settings portable. I mean, we commit other stuff in the .idea folder, I feel like this should work. I wonder if we need to do something like we do for the Gradle JVM settings and modify the |
So it turns out you can configure the imports order in IntelliJ via To see all supported properties, right-click a folder and select "New | EditorConfig File", check the IntelliJ IDEA-specific checkbox in the dialog that opens and, if needed, select the languages used in your project. An |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Brilliant 👍
Change the formatter config to sort / order imports, and reformat the codebase. We already had a config file for Eclipse users, so Spotless now uses that. The "Eclipse Code Formatter" plugin ought to be able to use this file as well for import ordering, but in my experiments the results were poor. Instead, use IntelliJ's `.editorconfig` support to configure import ordering. I've also added a config file for the formatter plugin. Other changes: * I've quietly enabled the `toggleOnOff` option for Spotless. It was already possible to disable formatting for sections using the markers for docs snippets, so enabling this option just accepts this reality and makes it possible via `formatter:off` and `formatter:on` without the restrictions around line length. It should still only be used as a very last resort and with good reason. * I've removed mention of the `paddedCell` option from the contributing guide, since I haven't had to use that option for a very long time. I moved the docs to the spotless config.
Backported to 7.x in fb8f84f |
* master: (284 commits) [DOCS] Update central reporting image (elastic#74195) [DOCS] SQL: Document `null` handing for string functions (elastic#74201) Fix Snapshot Docs Listing Query Params in Body Incorrectly (elastic#74196) [DOCS] EQL: Note EQL uses `fields` parameter (elastic#74194) Mute failing MixedClusterClientYamlTestSuiteIT test {p0=indices.split/20_source_mapping/Split index ignores target template mapping} test (elastic#74198) Cleanup Duplicate Constants in Snapshot XContent Params (elastic#74114) [DOC] Add watcher to the threadpool doc (elastic#73935) [Rest Api Compatibility] Validate Query typed api (elastic#74171) Replace deprecated `script.cache.*` settings with `script.context.$constext.cache_*` in documentation. (elastic#74144) Pin Alpine Linux version in Docker builds (elastic#74169) Fix clone API settings docs bug (elastic#74175) [ML] refactor internal datafeed management (elastic#74018) Disable query cache for FunctionScoreQuery and ScriptScoreQuery (elastic#74060) Fork the sending of file chunks during recovery (elastic#74164) RuntimeField.Builder should not extend FieldMapper.Builder (elastic#73840) Run CheckIndex on metadata index before loading (elastic#73239) Deprecate setting version on analyzers (elastic#74073) Add test with null transform id in stats request (elastic#74130) Order imports when reformatting (elastic#74059) Move deprecation code from xpack core to deprecation module. (elastic#74120) ...
Change the formatter config to sort / order imports, and reformat the codebase. We already had a config file for Eclipse users, so Spotless now uses that.
The "Eclipse Code Formatter" plugin ought to be able to use this file as well for import ordering, but in my experiments the results were poor. Instead, use IntelliJ's
.editorconfig
support to configure import ordering.I've also added a config file for the formatter plugin.
Other changes:
toggleOnOff
option for Spotless. It was already possible to disable formatting for sections using the markers for docs snippets, so enabling this option just accepts this reality and makes it possible viaformatter:off
andformatter:on
without the restrictions around line length. It should still only be used as a very last resort and with good reason.paddedCell
option from the contributing guide, since I haven't had to use that option for a very long time. I moved the docs to the spotless config.