-
-
Notifications
You must be signed in to change notification settings - Fork 447
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
Only need to run lint before push. #2327
Comments
imo, we want every commit to be runnable and without errors. If the lint is run before pushing instead of before committing we have two options. Make a commit that fixes the potential issues, or revert commits to fix them. The first is bad because it means the previous commits have warnings and maybe even errors in them, and the second option is bad because it is potentially a lot more work than just running the lint before each commit. What you can do if you really do not want to run the lint for a specific commit (it's tiny, you are 100% certain there are no errors, or it's a temporary commit) is to do the commit with the |
@Frans-Lukas commits are not necessarily on the repo though. I can't speak to others style of writing but I would nearly always commit and push at the same time so logistically this makes zero difference to me. If a person has a bunch of local commits, tries to push and then it fails they can happily rebase and rework their local history before making it the history of the repo. @gouri-panda it might help if you tell us about your workflow. |
When I work on an issue. I generally commit small meaningful changes, after the issue is solved i push to my repo. |
I am happy for this ticket to be accepted, it will have to delete the commit hook as part of the install task. The install task is located in |
This changes the behaviour, the documentation does not make this irrelevant |
This issue has been automatically marked as stale because it has not had recent activity. It will be now be reviewed manually. Thank you for your contributions. |
@gouri-panda After 3 years, I finnaly fully read this issue and the comments. I fully agree with @Frans-Lukas. The multiples commits of a PR are not try an errors, they are a way to split an enhancement in multiple logicial small parts. Each part should be linted for quality reasons. |
@kelson42 Yup! Couldn’t agree more. At that time I didn't know the value of each commits with lint checks. It's a trade-off between time & efficiency. But in a long run, we can see significant benefits from it! |
Now lint is running every commit. It will be much easier to run lint if it runs only before the push.
The text was updated successfully, but these errors were encountered: