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

Conversation

Projects
None yet
5 participants
@alexsasharegan
Contributor

alexsasharegan commented Aug 10, 2017

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

@codecov-io

This comment has been minimized.

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.

@tmorehouse tmorehouse requested a review from mosinve Aug 10, 2017

"prefer-reflect": "off",
"prefer-template": "off",
"quote-props": "off",
quotes: "off",

This comment has been minimized.

@mosinve

mosinve Aug 10, 2017

Member

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

Alex Regan
'commonjs': true,
'es6': true
env: {
browser: true,

This comment has been minimized.

@mosinve

mosinve Aug 10, 2017

Member

Why here not using quotes?
this breaks uniformity

This comment has been minimized.

@alexsasharegan

alexsasharegan Aug 10, 2017

Contributor

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.

This comment has been minimized.

@alexsasharegan

alexsasharegan Aug 10, 2017

Contributor

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 😜

This comment has been minimized.

@mosinve

mosinve Aug 10, 2017

Member

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 }]

This comment has been minimized.

@tmorehouse

tmorehouse Aug 10, 2017

Member

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)

This comment has been minimized.

@alexsasharegan

alexsasharegan Aug 10, 2017

Contributor

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

This comment has been minimized.

@mosinve

mosinve Aug 10, 2017

Member

doublequotes are default setting for eslint

Alex Regan

@pi0 pi0 merged commit c33d1d4 into dev Nov 17, 2017

0 of 2 checks passed

License Compliance FOSSA analyzing commit
Details
ci/circleci CircleCI is running your tests
Details

@tmorehouse tmorehouse deleted the feat/eslint-settings branch Nov 17, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment