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

(ci): add a lint job so PRs will require passing lint #378

Merged
merged 1 commit into from
Dec 18, 2019

Conversation

agilgur5
Copy link
Collaborator

@agilgur5 agilgur5 commented Dec 18, 2019

  • 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

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 for tsdx lint and that runs over the matrix ofc.

- 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
@agilgur5
Copy link
Collaborator Author

agilgur5 commented Dec 18, 2019

Ok, this is a bit weird that it passes lint prior to #377 being merged...

The error I was getting was:

yarn run v1.19.1
$ yarn build && yarn lint:post-build
$ tsc -p tsconfig.json
$ node dist/index.js lint src test --ignore-pattern 'test/tests/lint'
=============

WARNING: You are currently running a version of TypeScript which is not officially supported by typescript-estree.

You may find that it works just fine, or you may not.

SUPPORTED TYPESCRIPT VERSIONS: >=3.2.1 <3.7.0

YOUR TYPESCRIPT VERSION: 3.7.3

Please only submit bug reports when using the officially supported version.

=============

~/Desktop/GitHub/tsdx/test/fixtures/build-default/src/syntax/nullish-coalescing.ts
  5:24  error  Parsing error: Expression expected  prettier/prettier

~/Desktop/GitHub/tsdx/test/fixtures/build-default/src/syntax/optional-chaining.ts
  3:52  error  Parsing error: Expression expected  prettier/prettier

✖ 2 problems (2 errors, 0 warnings)

error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

But then the prettier errors stopped happening once I added the fix, including on local, despite that not being part of this branch... 🤔

@swyxio
Copy link
Collaborator

swyxio commented Dec 18, 2019

great calls in every respect.

@swyxio swyxio merged commit 930feb9 into jaredpalmer:master Dec 18, 2019
@agilgur5
Copy link
Collaborator Author

Ah, I see, #370 added a commit later which bumped prettier which bumped its dependency on @typescript-eslint/typescript-estree, so it then started to pass lint when deps were upgraded.

#377 was still needed to get rid of the big warning and to support optional chaining / nullish coalescing in eslint proper (not just in prettier), but not to fix the specific prettier errors I wrote of above.

sebald added a commit to sebald/tsdx that referenced this pull request Dec 19, 2019
* 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
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants