-
Notifications
You must be signed in to change notification settings - Fork 25
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
Add .coafile and Travis lint job #59
base: master
Are you sure you want to change the base?
Conversation
}, | ||
"rules": { | ||
"no-unused-vars": 1, | ||
"react/jsx-uses-vars": 1, | ||
"no-var": 2, | ||
"new-cap": 0, | ||
"quotes": [1, "single", "avoid-escape"], | ||
"semi": 1, | ||
"indent": [2, 2] |
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.
Removing this is annoying ... as the rule is generally ok, but there are a bunch of exceptions which probably need to be added to get it working. (help ...?)
An alternative is to not ugprade eslint to 4.19. I chose that version because it is the same as being used on git-task-list, and IMO we should aim at having the same in our JS repos.
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.
Airbnb style actually uses 4.9 as peer dependency. I really want to downgrade to that matching version :( too. Waiting for @blazeu .
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.
Was using 4.19 as i though, later is better. (it was before adding airbnbstyleguide)
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.
Airbnb style actually uses 4.9 as peer dependency.
No it doesnt @bekicot.
It uses eslint: '^4.9.0'
, which in case you didnt know means any version of 4, after that.
To check what a JS semvar spec means, https://semver.npmjs.com/ is very helpful.
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.
4.9 seems to work fine and no additional overhead.
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.
I know it would work with 4.19 with no dependency error. But it add additional overhead.
.coafile
Outdated
|
||
[all.js] | ||
bears = ESLintBear | ||
eslint_config = .eslintrc.json |
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.
.eslintrc
?
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.
Though .eslintrc
is deprecated: https://eslint.org/docs/user-guide/configuring#configuration-file-formats
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.
I was toying with the idea of changing the format of this file, but it made the diff harder to read because lots of other changes were needed to the file. And then forgot to change this back. Thanks for spotting.
After this is merged we should update these in all repos. Yaml would be my preference.
@@ -42,5 +67,5 @@ deploy: | |||
provider: pages | |||
skip_cleanup: true | |||
github_token: $GITHUB_TOKEN | |||
on: | |||
"on": |
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.
I vaguely remember doing something like this in the past, but my quick searches couldn't come up with anything. What's the story behind this?
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.
it is from the yaml lint bear.
I used to disabled it in git-task-list. But it is far easier just to quote it.
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.
I guess you did it in gci-leader?
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.
yup @andrewda did. coala/gci-leaders@dff8ba5
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.
Haha, I knew that was familiar 😂
Update .eslintrc to remove deprecated emcaFeatures and remove indent rule which fails with newer eslint.
@gitmate-bot rebase |
Hey! I'm GitMate.io! This pull request is being rebased automatically. Please DO NOT push while rebase is in progress or your changes would be lost permanently |
Automated rebase failed! Please rebase your pull request manually via the command line. Reason:
|
Closes #48