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

feat(eslint): update settings to remove editor errors #792

Merged
merged 4 commits into from Nov 17, 2017
Merged

Conversation

alexsasharegan
Copy link
Member

As stated, just trying to remove all the red squigglies from my editor.

@codecov-io
Copy link

codecov-io commented Aug 10, 2017

Codecov Report

Merging #792 into dev will decrease coverage by 0.4%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##            dev    #792      +/-   ##
=======================================
- Coverage    40%   39.6%   -0.41%     
=======================================
  Files        68      68              
  Lines      2217    2207      -10     
  Branches    633     632       -1     
=======================================
- Hits        887     874      -13     
  Misses     1171    1171              
- Partials    159     162       +3
Impacted Files Coverage Δ
lib/utils/observe-dom.js 0% <0%> (-60%) ⬇️
lib/components/form-radio.vue 40.74% <0%> (-5.1%) ⬇️
lib/components/carousel-slide.vue 62.5% <0%> (-4.17%) ⬇️
lib/components/carousel.vue 34.02% <0%> (-3.6%) ⬇️
lib/components/alert.vue 62.16% <0%> (+0.62%) ⬆️
lib/components/form-checkbox.vue 41.66% <0%> (+1.12%) ⬆️
lib/components/collapse.vue 68.18% <0%> (+2%) ⬆️

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 0c2ca85...83c83f1. Read the comment docs.

.eslintrc.js Outdated
"prefer-reflect": "off",
"prefer-template": "off",
"quote-props": "off",
quotes: "off",
Copy link
Member

Choose a reason for hiding this comment

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

If you use doublequotes, maybe to enable this rule?
quotes: [2, "double", "avoid-escape"]

'commonjs': true,
'es6': true
env: {
browser: true,
Copy link
Member

Choose a reason for hiding this comment

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

Why here not using quotes?
this breaks uniformity

Copy link
Member Author

Choose a reason for hiding this comment

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

Because Prettier did it. It only escapes illegal object key identifiers. I personally like this, but ultimately, I just go with whatever the tool does for me. Had it maintained consistency, I would have deferred to that. I will say this: prettier is a fairly accurate representation of the majority consensus of js style. In other words: you may not like it, but most people do.

Copy link
Member Author

Choose a reason for hiding this comment

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

My personal reason for appreciating dropping quoting object keys that are legal identifiers: it enables syntax highlighting. For me, string syntax highlighting gives me no indication of what the token is. I like being able to tell it's an object key. Not a big deal to me either way though. I just hit format, and move on. Guilty 😜

Copy link
Member

Choose a reason for hiding this comment

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

there's eslint rule about this case - "quote-props"
currently it switched off, my offer to set it to: "quote-props": [2, "as-needed", { "keywords": true }]

Copy link
Member

Choose a reason for hiding this comment

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

I use double quotes always in JSON object files, since quotes preferred over double quotes within source code around strings (unless the string has a lot of escaped single quotes), and literals for keys, unless they have unsupported characters.

Everyone has a personal taste ;-)

I find single quotes make the file lighter than double quotes (less black ink, heheh)

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm just trying to let my OCD go to my tooling. Of course, then my tooling OCD becomes a problem...

Copy link
Member

Choose a reason for hiding this comment

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

doublequotes are default setting for eslint

@pi0 pi0 merged commit c33d1d4 into dev Nov 17, 2017
@tmorehouse tmorehouse deleted the feat/eslint-settings branch November 17, 2017 22:30
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.

None yet

5 participants