[canvas] remove unnecessary eslint style overrides, use curlys#27176
[canvas] remove unnecessary eslint style overrides, use curlys#27176spalger merged 5 commits intoelastic:masterfrom
Conversation
spalger
added
chore
Team:Presentation
|
Pinging @elastic/kibana-canvas |
Awesome 😻! That's been really frustrating for everyone. The output here looks consistent too, which is great! I haven't really poured over the changes here, but a quick spot check looks good. Is this meant to replace #27083 then? |
|
Yep 👍 |
|
Cool. I'm not going to be able to get to this until Monday probably, but if @stacey-gammon and @monfera are on board, there's no need to wait for me. |
💚 Build Succeeded |
stacey-gammon
left a comment
stacey-gammon
left a comment
There was a problem hiding this comment.
lgtm, code reviewed the config changes, skimmed the other changes. did not pull down and test manually, but am assuming tests will catch any obvious errors.
| 'packages/kbn-test/**/*', | ||
| 'packages/kbn-eslint-import-resolver-kibana/**/*', | ||
| 'x-pack/plugins/apm/**/*', | ||
| 'x-pack/plugins/canvas/**/*', |
There was a problem hiding this comment.
how come things like src/* aren't in here? or other plugins like reporting?
There was a problem hiding this comment.
Prettier support is opt-in, we wanted to "take it slow" which basically just meant enabling it for one piece at a time when someone wanted to go through the pain of comiting the massive autofix pr.
There was a problem hiding this comment.
Does opting into Prettier here make the prettier config in the Canvas overrides redundant? It would be nice if that stuff was only configured once.
There was a problem hiding this comment.
Yep, thought I removed all of that but I had left behind the plugin: ['prettier'] bit, removed it now.
| plugins: ['prettier'], | ||
| rules: { | ||
| // preferences | ||
| 'comma-dangle': [2, 'always-multiline'], |
There was a problem hiding this comment.
Why'd you remove the comma dangle and spacing rules? Perhaps, they're upstream rules now and so they're redundant here?
There was a problem hiding this comment.
Yeah, they're handled by prettier.
w33ble
left a comment
w33ble
left a comment
There was a problem hiding this comment.
Fix output looks good. Still curious about the comma dangle and spacing rules being removed, but either way the changes here LGTM.
This comment has been minimized.
This comment has been minimized.
…ents/datetime_range_absolute/datetime_range_absolute.js
💔 Build Failed |
|
Job succeeded but asset upload failed |
…ic#27176) * [canvas] remove styling rules that are handled by prettier, always use curlys in if * [eslint] autofix missing curly brackets * [eslint/canvas] remove redundant prettier plugin config * autofix lint errors in canvas_plugin_src/renderers/time_filter/components/datetime_range_absolute/datetime_range_absolute.js
…ic#27176) * [canvas] remove styling rules that are handled by prettier, always use curlys in if * [eslint] autofix missing curly brackets * [eslint/canvas] remove redundant prettier plugin config * autofix lint errors in canvas_plugin_src/renderers/time_filter/components/datetime_range_absolute/datetime_range_absolute.js
… (#27355) * [canvas] remove styling rules that are handled by prettier, always use curlys in if * [eslint] autofix missing curly brackets * [eslint/canvas] remove redundant prettier plugin config * autofix lint errors in canvas_plugin_src/renderers/time_filter/components/datetime_range_absolute/datetime_range_absolute.js
4 participants
Based partially on the changes in #27083, this PR removes the style rules that we migrated over from when canvas was its own repo. Best I can tell they are not necessary anymore as the
eslint-config-prettierconfig has gotten a lot better at filtering out conflicting rules as implemented in the general "enable prettier" override section.I spot checked rules like
comma-dangle,no-multiple-empty-lines,wrap-iife, etc and they all seem to work as expected by some extra checks from @w33ble and @monfera would be appreciated.My primary motivation for this change is currently in master, whenever I save a file multiple
;or brace changes are made that take multiple saves to resolve and sometimes never do. The changes in this PR bring canvas a little more inline with the rest of Kibana which makes contributing to the plugin a little more pleasant for me.