-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
chore: use prettier for lint #1362
Conversation
83d5ddf
to
0fd92b8
Compare
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.
Could we add a pre-commit linter hook here?
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.
👍 for consistency. Agreed with Erick, we should add something like husky
+ lint-staged
.
npm ci | ||
npm test |
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.
Maybe for another PR, but generally I've put npm ci
and npm test
in different run steps, but if you want to do it in one step:
npm ci | |
npm test | |
npm install-ci-test |
if: github['ref'] == 'refs/heads/master' | ||
- name: Use Node.js 10.x | ||
if: github['ref'] == 'refs/heads/master' | ||
uses: actions/setup-node@v1 | ||
with: | ||
node-version: 10.x | ||
- name: npm install, build, and test | ||
if: github['ref'] == 'refs/heads/master' |
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.
Again, maybe for another PR, but shouldn't the branch check be on the on: push
instead of on each step?
on:
push:
branches:
- master
It's pretty old config and I plan to update them to standard web infra config in refactor PR. Added EDIT: Also we maybe need a request update the required checks. |
Does what it says on the tin.
Also enabled lint for
yaml
files so every app is similar to others.