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

Lint workflow #6

Merged
merged 9 commits into from
Jan 11, 2021
Merged

Lint workflow #6

merged 9 commits into from
Jan 11, 2021

Conversation

richardolsson
Copy link
Member

This PR improves the developer experience around linting and type-checking. It does so in a number of ways.

Improvements

Less disruptive linting and type-checking

Linting will no longer raise build-preventing errors. This makes it ok to have temporary issues in your code while you're developing, as long as you fix them before committing.

The TypeScript type-checker does not run during NEXTjs development to avoid disrupting the development process. The philosophy is sane, but it did mean that it would never run with our existing setup, so our code actually contained errors which we weren't noticing.

With this PR, docker-compose up will spin up a separate container which just does typescript compilation and linting, alongside but separate from the NEXTjs development server. This means that we get linting and typescript errors in the terminal, but without disrupting the development flow.

Standalone linting and pre-commit hook

The linting can now be run standalone using docker-compose run main npm run lint (or as part of the existing main container using docker-compose exec main npm run lint.

This command can also be used as a pre-commit hook, so that you never forget to verify style and type safety before committing code. This PR adds a hook in .githooks/pre-commit and an explanation in the README on how to configure it.

image

How to test

  1. Check out the branch
  2. Configure the git hook if you want (recommended, git config core.hooksPath .githooks)
  3. Rebuild and restart the docker-composition (docker-compose up --build)
  4. Edit some code, introducing a style or type issue, e.g. declaring a variable that is never used or adding console.log() somewhere
  5. Try committing your code (git commit -a)

Expected result:

After step 4, the linter container should print out an error in the terminal. The code should still run ok in the browser.

After step 5, another linter container should start up and run the eslint src && tsc commands to validate the code. It should fail (due to the bad code introduced in step 4) and the commit should be prevented.

@richardolsson
Copy link
Member Author

Oh I forgot to mention, but the PR also fixes a number of linting and type issues which previously weren't being caught, or which were introduced in another branch before more linting was introduced.

Curious to hear your thoughts @niklasva82 and @kristofferlarberg!

@richardolsson richardolsson merged commit 1272b59 into main Jan 11, 2021
@richardolsson richardolsson deleted the lint-workflow branch January 21, 2021 14:01
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.

1 participant