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 eslint #280

Open
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@drewfreyling
Copy link
Member

commented Jun 5, 2019

fix current eslint issues:

  • around space-before-function
  • paren and no-unused-vars

Fixes #174

@drewfreyling

This comment has been minimized.

Copy link
Member Author

commented Jun 5, 2019

Once/if this is approved, we should add npx eslint to the CI pipeline.

Show resolved Hide resolved webServer.js Outdated
Show resolved Hide resolved webServer.js
Show resolved Hide resolved webServer.js Outdated

@drewfreyling drewfreyling force-pushed the drewfreyling:feature/eslint branch from 21be39f to 82896f6 Jun 5, 2019

@drewfreyling drewfreyling force-pushed the drewfreyling:feature/eslint branch from 82896f6 to 14d86b3 Jun 5, 2019

@drewfreyling drewfreyling self-assigned this Jun 6, 2019

@MattIPv4
Copy link
Member

left a comment

Lgtm.

@drewfreyling

This comment has been minimized.

Copy link
Member Author

commented Jun 7, 2019

Lgtm.

Don't seem to have permissions anymore. I've requested access to the dev team (which i used to have) for this.

@MattIPv4

This comment has been minimized.

Copy link
Member

commented Jun 7, 2019

@drewfreyling I'm part of the dev team and can't merge either.
cc @PeterDaveHello ?

Show resolved Hide resolved .eslintrc.json Outdated
@PeterDaveHello

This comment has been minimized.

Copy link
Member

commented Jun 9, 2019

I think @cdnjs/dev do have write access to this repo?

@PeterDaveHello
Copy link
Member

left a comment

LGTM

Show resolved Hide resolved .eslintrc.json Outdated
@MattIPv4

This comment has been minimized.

Copy link
Member

commented Jun 12, 2019

cc #174

@drewfreyling drewfreyling requested a review from PeterDaveHello Jun 14, 2019

@MattIPv4

This comment has been minimized.

Copy link
Member

commented Jun 14, 2019

@drewfreyling Is it worth compacting your two commits down into one, dropping the merge commit and then rebasing against master, for a cleaner history?

@drewfreyling

This comment has been minimized.

Copy link
Member Author

commented Jun 14, 2019

Why don't we just squash and merge?

All changes resolved.

@drewfreyling drewfreyling requested a review from MattIPv4 Jun 17, 2019

@MattIPv4
Copy link
Member

left a comment

Lgtm (haven't done a test run of eslint as away from laptop)

@drewfreyling drewfreyling removed the request for review from PeterDaveHello Jun 19, 2019

@drewfreyling

This comment has been minimized.

Copy link
Member Author

commented Jun 19, 2019

@MattIPv4 @PeterDaveHello cant' merge this - "The base branch restricts merging to authorized users. Learn more about protected branches."

@MattIPv4

This comment has been minimized.

Copy link
Member

commented Jun 19, 2019

@drewfreyling I can't merge either, only @PeterDaveHello can

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.