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

refactor: Replace husky & lint-staged by lefthook #330

Closed
wants to merge 1 commit into from

Conversation

alimtunc
Copy link
Contributor

Running pre-commit fast, concurrently and only on staged files! (Even for mypy now 🔥)

Capture d’écran 2023-08-28 à 10 58 15

Copy link
Contributor

@ramnes ramnes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

commands that modify stuff running in parallel is probably a bad idea

@alimtunc
Copy link
Contributor Author

commands that modify stuff running in parallel is probably a bad idea

Depends on how they synchronize the updated files, I did not see issue like that. But yeah, need to take a long in a long term and modified base.

But normally, you shouldn't have stuff being modified by lefthook since your dev workspace should be setup and lint/format on each files update 🫣

@ramnes
Copy link
Contributor

ramnes commented Aug 28, 2023

Depends on how they synchronize the updated files, I did not see issue like that.

there is no synchronization, that's the problem

But yeah, need to take a long in a long term and modified base.

I don't understand this sentence

But normally, you shouldn't have stuff being modified by lefthook since your dev workspace should be setup and lint/format on each files update 🫣

then we should switch these tools to their "check-only" mode, but as far as I'm concerned, I do rely on pre-commit to format the files and I'm not sure to see why I should not

@willydouhard
Copy link
Collaborator

I kinda agree with @ramnes. Some developers will lint on save some rely on the pre commit hook.
The concurrency point is also valid, if they the different steps are ran concurrently what happens if they modify the same file at the "same" time?

@alimtunc
Copy link
Contributor Author

alimtunc commented Aug 28, 2023

there is no synchronization, that's the problem

Alright, I haven't come across anyone encountering problems with it. However, if you still have concerns, what about grouping these by file types?

I don't understand this sentence

Sorry, I also didn't quite understand my sentence initially, haha. I meant that we should consider the long-term usage of these tools and see if any issues arise :)

then we should switch these tools to their "check-only" mode, but as far as I'm concerned, I do rely on pre-commit to format the files and I'm not sure to see why I should not

Pre-commit is, to me, a way to ensure that I don't commit poor-quality code to the repository. As a developer, it's advisable to have tools like Prettier, linters, Black, and isort set up to automatically run whenever you save your files. This practice ensures that your code remains properly formatted and compliant with coding standards throughout your work. Embracing these practices offers benefits, and pre-commit serves as a final checkpoint, verifying the quality of your code before commits.

However, it's possible that my perspective is unique, and others may have differing opinions. In my view, you shouldn't just rely on pre-commit.

@alimtunc
Copy link
Contributor Author

Ok, I ended up with something like this. Let me know @ramnes @willydouhard

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