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

Configure linter and run it on CI #71

Closed
2 tasks
olizilla opened this issue Oct 29, 2018 · 6 comments · Fixed by #338
Closed
2 tasks

Configure linter and run it on CI #71

olizilla opened this issue Oct 29, 2018 · 6 comments · Fixed by #338
Assignees

Comments

@olizilla
Copy link
Collaborator

olizilla commented Oct 29, 2018

Comming from standard js world, I'd expect double quotes for strings to get picked up by the lint script. This came up in a PR #63 (comment)

We need to

  • add npm run lint to the travis config so we get feedback on PRs
  • figure out why the vue-cli-service lint reports no lint errors when mixed quote styles are used.
@olizilla olizilla mentioned this issue Oct 29, 2018
@terichadbourne
Copy link
Member

Is it possible this particular case wasn't picked up as an error by the linter because the string had an apostrophe inside it and couldn't have been produced with single quotes surrounding it? If so, maybe there's a way to add our own rule to require ` instead of " around the string in that case?

@mikeal
Copy link
Member

mikeal commented Oct 30, 2018

Is it possible this particular case wasn't picked up as an error by the linter because the string had an apostrophe inside it and couldn't have been produced with single quotes surrounding it?

This is absolutely why :)

@terichadbourne
Copy link
Member

terichadbourne commented Oct 31, 2018 via email

@mikeal
Copy link
Member

mikeal commented Oct 31, 2018

This still needs to be tackled: " add npm run lint to the travis config so we get feedback on PRs"

However, it's a little more involved than that. Our current travis config is for gh-pages deploys so it doesn't run on every PR, it only runs on merges to master.

@terichadbourne
Copy link
Member

May I assign this one to you @mikeal so I can stay focused on site content for the time being? (Happy to have you grab me when you're doing it if you want to show me how it works; I don't know anything about this kind of plumbing.)

@mikeal
Copy link
Member

mikeal commented Nov 1, 2018

Sure, I'll assign it to myself to save you the hassle :)

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 a pull request may close this issue.

3 participants