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

Add .coafile and Travis lint job #59

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Add .coafile and Travis lint job #59

wants to merge 4 commits into from

Conversation

jayvdb
Copy link
Member

@jayvdb jayvdb commented Jun 13, 2018

Closes #48

},
"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]
Copy link
Member Author

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.

Copy link

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 .

Copy link

@bekicot bekicot Jun 13, 2018

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)

Copy link
Member Author

@jayvdb jayvdb Jun 16, 2018

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.

Copy link

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.

Copy link

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
Copy link
Member

Choose a reason for hiding this comment

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

.eslintrc?

Copy link
Member

Choose a reason for hiding this comment

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

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 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":
Copy link
Member

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?

Copy link

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.

Copy link

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?

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

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.
@jayvdb
Copy link
Member Author

jayvdb commented Jun 23, 2018

@gitmate-bot rebase

@gitmate-bot
Copy link

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 ⚠️

@gitmate-bot
Copy link

Automated rebase failed! Please rebase your pull request manually via the command line.

Reason:

Command: git rebase base/master

Exit Code: 128

Cause:

error: Failed to merge in the changes.


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

Successfully merging this pull request may close these issues.

5 participants