-
Notifications
You must be signed in to change notification settings - Fork 508
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
(ci): add a lint job so PRs will require passing lint #378
Conversation
- a few recent PRs got in without passing lint, this breaks all future PRs/branches/commits because pre-commit will fail when it runs lint
Ok, this is a bit weird that it passes lint prior to #377 being merged... The error I was getting was:
But then the |
great calls in every respect. |
Ah, I see, #370 added a commit later which bumped #377 was still needed to get rid of the big warning and to support optional chaining / nullish coalescing in |
* master: (26 commits) (deps/lint): upgrade @typescript-eslint to support ?. and ?? (jaredpalmer#377) (ci): add a lint job so PRs will require passing lint (jaredpalmer#378) (clean): remove .rts_cache_* from storybook gitignore (jaredpalmer#375) Add optional chaining and nullish coalescing operators support (jaredpalmer#370) Added Storybook template (jaredpalmer#318) (fix/ci): GitHub Actions should run on PRs as well (fix/format): formatting of jaredpalmer#366 didn't pass lint Add prepare script to generated project (jaredpalmer#334) default jest to watch mode when not in CI (jaredpalmer#366) (fix): respect tsconfig esModuleInterop flag (jaredpalmer#327) fix: minor typo update rollup deps and plugins update to ts 3.7 Remove unnecessary yarn install command in GH action update README.md update README.md Use node_modules/.cache/... as cacheRoot (jaredpalmer#329) fix(lint): Only default to src test if they exist (jaredpalmer#344) Fix error when providing babel/preset-env without options (jaredpalmer#350) Replaced some sync methods for their async version ...
PRs/branches/commits because pre-commit will fail when it runs lint
Turns out
lint
was never running in CI! Which explains why #370 and #366 passed (and my confusion in #373 )This should make formatting fixes like #372 and #377 unnecessary in the future if only PRs that pass lint are merged.
I limited this to run on Node v10 and
ubuntu-latest
-- not sure if a matrix is necessary for self-linting. The existing test suite already has tests fortsdx lint
and that runs over the matrix ofc.