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

Git pre-push hook to lint --fix the project #590

Closed
4 tasks done
andretchen0 opened this issue Mar 15, 2024 · 9 comments
Closed
4 tasks done

Git pre-push hook to lint --fix the project #590

andretchen0 opened this issue Mar 15, 2024 · 9 comments
Labels
p2-nice-to-have Not breaking anything but nice to have (priority)

Comments

@andretchen0
Copy link
Contributor

andretchen0 commented Mar 15, 2024

Description

As a developer using TresJS ...

Problem

I forget to lint --fix before pushing. Others do too. This leads to messier commit histories, noisier diffs, and wasted time.

Suggested solution

Add a client-side pre-push git hook to lint --fix the project and abort the push if errors exist.

By default, the hook would run lint --fix during a push, before any objects have been transferred.

If there are no linter errors after running lint --fix, it would:

  • continue the push

If there are unfixable-by-the-linter errors, it would:

  • abort the push
  • report the errors (or filenames with errors) and instruct the user to fix them
  • give the user instructions for bypassing the linter step and pushing despite errors

Advantages

  • Reduce the number of PRs that fail the CI lint step
  • Reduce the amount of diff "noise" due to chore(lint): fix linter errors
  • Reduce mental load when pushing – no need to remember to lint

Disadvantages

  • Some new contributors might be turned away. E.g. maybe the linter fails and they can't figure out the next step.
  • If linter errors make it into main, everyone's git push based on the branch will initially fail.
  • pnpm lint --fix takes a while to run.

Alternative

Do nothing. Keep the project as-is.

Additional context

The project runs a linter as a CI step and fails unlinted PRs already.

Validations

@alvarosabu
Copy link
Member

Sounds good, I'm planning to refactor @tresjs/eslint-config to extend https://github.com/antfu/eslint-config flat config and using ESLint Stylistic. I did it with my personal eslint config last week to test it and it works fine.

@alvarosabu
Copy link
Member

Here is the task Tresjs/configs#4

@damienmontastier
Copy link

damienmontastier commented Mar 18, 2024

Hey @alvarosabu. Wouldn't using ESLint Vue or extending it be more viable for Vue's "good practices"?

@alvarosabu
Copy link
Member

alvarosabu commented Mar 18, 2024

Hi @damienmontastier Antfu's config extends the eslint vue rules with some opinionated overrides (which I agree) https://github.com/antfu/eslint-config/blob/main/src/configs/vue.ts

You can check the tres eslint config here https://github.com/Tresjs/eslint-config

@andretchen0
Copy link
Contributor Author

Hey @alvarosabu !

  • I've got a working solution. It uses the husky plugin. Is that ok with you?
  • Is this something we want to eventually add to other projects in the ecosystem? If so, I'll start with Cientos as it's where I'm working right now.

@alvarosabu
Copy link
Member

Hi, @andretchen0 sounds good, so the pre-commit of husky will lint fix then?

@alvarosabu alvarosabu added the p2-nice-to-have Not breaking anything but nice to have (priority) label Mar 30, 2024
@andretchen0
Copy link
Contributor Author

Hi, @andretchen0 sounds good, so the pre-commit of husky will lint fix then?

Yeah.

Husky is a plugin that makes git hook configuration a bit more straightforward.

The usual git commit, push, etc. commands won't change. They'll trigger a git hook configured by husky.

@andretchen0
Copy link
Contributor Author

andretchen0 commented Apr 7, 2024

@andretchen0
Copy link
Contributor Author

Closing here on TresJS as there's now a PR on Cientos. Continuing the discussion there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p2-nice-to-have Not breaking anything but nice to have (priority)
Projects
Status: Done
Development

No branches or pull requests

3 participants