Tried this on a python dev team. Led to team members committing less often, as each commit kicked off a small battle with the autoformatter and linter. Refactor code tickets were the worst. You comment out big chunks of functionality temporarily to keep things working through the transition, then the formatter removes all the orphaned variables/imports, which can take hours to reproduce. The sweet spot in my opinion is to enforce lint before merging a PR into master.
Great to hear your experience, this is definitely something to be weary of. It sounds like you may have had your linter settings turned up too high, I would recommend setting it to only catch errors that you definitely don't want committed. But in any case there is definitely friction introduced by the process. If the friction is too much as a pre-commit hook, you can also specify the scripts to run at pre-push time so devs can work freely on their local copies without any friction.
+1, it's really better to put that in CI and enforce a good branching model with branch protection, status checks and code reviews on merge. Not a fan of adding dependencies, having to have the whole environment setup just to make a quick change, etc.
@@misterGaseg Is this what happens if one doesn't do any testing with the changes one has done before commiting? It seems that with CI developers may tend to leave all testing/verification up to the automated parts of the process...
@@benhetland576 The workflow I prefer is to add unit tests that exercise each new code path as I go. But a commit is not a merge. Commits should be free and painless, with very few exceptions (large files comes to mind as an exception). Sometimes I commit just because I need to start a ticket over, and want to save everything in case some of it winds up being useful later. I don't want to test/lint changes that aren't destined for master.
Doing that for small projects is ok, but imagine a large project where running all the tests (e2e, unit, integration, etc) take about an hour or more Unless there is way to only run tests related to the files that were changed
In my experience git hooks don't really work in practice since you can't enforce them properly. You need to nag each individual developer to install them every time they clone a new repository. What works much better is having a format check in CI and forbid merges unless all tests pass. Also, this has additional benefit of allowing developers to work at their own pace and only tidy up code when they are ready to merge.
I double that. The only way git hooks are useful is either you are working on a project alone or on a very small team, otherwise, it quickly gets out of hand. Getting every developer setting up hooks every time they move to a new project/repo or change the environment they work on is just not happening in Enterprise. As much as GitHooks are promising, as long as they are not part of the source code of the repo they not going to work for bigger teams.
The yaml config is part of the source code of the repo, so it's the same amount of work as telling devs to "pip install -r requirements.txt". Just add "pre-commit install" after that in their setup instructions. In fact, you could even make it auto-install when they "pip install -e ." the project by adding it as a dev install hook in the setup.py, or by telling them to "git clone --template" the project from a template that has the hooks installed. It seems like the real issue most people have is with the friction involved at commit time, specifically by a linter or formatter, not with the difficulty of setting it up. I would argue that there are plenty of commit hooks that _really_ should be done before committing, like checking that no large files, private keys, or AWS credentials are committed, or checking that no syntax errors in yaml, json, or Python code are committed. As for code formatting and linting, maybe moving those to pre-push hooks would lessen the burden. I find that littering the git tree with "fix lint warning, code format" commits makes git history/blame much less useful and catching those things at least before a push happens so commits can be amended or squashed helps a lot. Of course, you should still be using your CI hooks as well, pre-commit is by no means a substitute for that.
@@mCoding The way our business unit operates is that we do have a lot of developers, and they are often shuffled into different 5-20 man teams, and assigned to different projects, plus devs are all over the world now, but in the same time we only have single DevOps team supporting all the projects with DevOps stuff. My DevOps team did try to enforce git-hooks prior to that, it was even less setup than what you showcase in the video. We had our own scripts folder for various hooks sitting in the repo, and all developers needed to do after cloning for the 1st time was to update git config to point to our hooks folder instead of the default .git/hook folder. There was no need to install any additional modules, nor to do any additional initializations within the repo. We had to constantly remind and nag developers to enable usage of hooks, after a year of that we gave up. In regards to all the benefits of running pre-commit and other hooks - I am 100% on board and agree with you. But from experience within my company - if we have to rely on our developer's continuous mindfulness to enable some of our DevOps functionality, it's most likely not going to be 100% reliable.
@@konstantinzavgorodniy3098 Git hooks are a convenience for the developer and to encourage cleaner commit history. As mentioned, they do not substitute for CI jobs which also run these commands and prevent any merges on the remote if there are failures.
@@kevinkalanda9946 All CI-CD jobs are done on post factum basis, and a big chunk of that effort could be eliminated by ability to enforce git hook usage through the repo/version control itself.
Incredible video as usual. Just a note: there is an erroneous colon on line 28 of the yaml file, in the .src argument :) Keep up the good work, always looking forward to your videos!
Thanks! The colon is not erroneous, paths are colon separated (a common linux convention for variables like $PATH). So this variable says current dir and src dir (I copied it from either pytest or mypy's config 😉)
Very weird. I just double checked to be sure and the pre-commit did not fail the yaml check on my machine. Perhaps a copy paste error? That should be line 30, not 28.
@@mCoding Yeah it was 28 on my machine because I left out the comments. I appreciate you checking it again, the exact message I get is: `while scanning a plain scalar in ".pre-commit-config.yaml", line 28, column 12 found unexpected ':' in ".pre-commit-config.yaml", line 28, column 39` I'm sure I'll figure it out
Sounds like something I want to run every month or so, but once I learn from the several runs, I just don't see the point and I have to uninstall the hook. That is probably trivial, of course. But, sounds great for security - that is something you want to really check, especially on very sensitive code.
Totally understandable, but rather than committing less often, here are some things you can do to lessen the friction. (1) run pre-commit as a pre-push hook rather than pre-commit hook (2) don't install pre-commit as a hook at all (or "pre-commit uninstall") and just manually run "pre-commit run" when you want to run the checks (3) edit your linter/mypy config to only catch serious errors that you would not be okay with committing
Also, I have question regarding, how to get almost *close to pycharm* behaviour in *vscode* _(using linters, formatters, language server)_ Please recommend, or may be make a video about that?
Great content, like always. But I wouldnt use pre commit auto formatter, because of two reasons: 1. It may mess with the logic/what the code tries to achieve 2. Because of #1, I need to double check after auto-formatting If a new commit is not expensive in your organization, I’d fix these issues with isort-flake8-mypy tests with new/different commits and with more control. Because the new changes will/should only be related to formatting/typing and should not change the logic.
The black auto formatter should not mess with the code's logic, since it compares the ASTs before and after formatting to make sure the code does the same thing it did before. This could be an issue with auto formatters for other languages, though
Yeah black checks that the AST's of the before and after are identical (1 or 2 very tame exceptions to this), so unless you are doing source code introspection there's not really any danger. However, pre-commit can also just be used as a pre-push tool (installed as a pre-push hook) or a manual tool. Even if it was manual it still makes it easy to run all your checks in one command.
Hes James, nice Video. One thing to look out for with mypy and pre-commit is that pre-commit by default will only run mypy on the changed files. This is not ideal as your changes in one file will often affect code in other files. Moreover, since pre-commit runs each hook in it's own venv, mypy can't properly typecheck your code when you use external dependencies. These issues have bitten me once or twice in the past. There's a better way to configure the hook, but it’s a bit more complicated. There's a good blog post about it by Jared Khan, I recommend looking it up some time.
Pre-commit doesn't require GitHub, so your organization can host all the pre-commit hooks they want to pre-approve and host them on any infrastructure they want. Of course, this requires getting approval from within your org.
To me, for "pre-commit" hook to MODIFY files is the worst possible design. I could accept the pre-commit hook doing some checks, or running some reporting, but modify files?! Are you kidding me? I'd flip out.
Haha I understand your issue. Indeed, very few pre-commit hooks actually modify your files. Black/other formatters and end of line fixer are the only ones I know that I would actually use. I'm constantly using the autoformat option in my IDE so perhaps I'm too trusting of black, but it does verify the AST of your code is identical to the one it started with and I've never had an issue with it. I'd understand if you only wanted no-modification hooks to run though.
I tried using pre-commit hooks to format and test my code automatically, but there are two things bothering me : showing a "commit error" simply because your code had an extra line in a random file irks me, I wish there was a simple "hey I formatted your code, check it again". Second thing is time : on enterprise code, tests often take several minutes to execute ; waiting this long to commit disrupts the workflow, and makes me (and any contributor) commit less often, which I don't want. Does anyone have this experience, and maybe tips to improve it ?
The error is saying "hey I formatted your file, check it again". It has to be an error because the commit has to not happen. That said I wouldn't mind an option to commit automatically after formatting, though I get that many people wouldn't want that.
pre-commit only checks the files that were staged for committing, so it should be really quick. Also, you (intentionally) don't have a unit test hook in pre-commit and you should run the tests independently, probably less often (e.g. after several commits & before push)
Import point I forgot to mention in the video! Pre-commit is not the place to run your tests. If you look at the list of pre-commit hooks, you'll notice that there aren't any for running tests. This is precisely for the reason you mentioned, tests might take a long time. You should still run your tests manually before committing, or just let your CI run them after committing. As for whether it should call a failed commit an "error" vs just tell you "files were changed by pre-commit hooks, commit again after inspecting", that's a great thing to suggest on their github issues page!
A hacker give this code to you def password(str_input, str_key): -> int Password_num = 0 str_key_len=len(str_key) str_input_len=len(str_input) for i in range(str_input_len): Password_num += str_key.index(str_input[i])*str_key_len**(str_input_len-i-1) return Password_num This function can take a string and output the password needed to protect the string Puzzle: Make a inverted function where argument are password and str_key Input: Line 1: password Line 2: str_key Output: A string Constrains: str_key cannot have duplicate letter and must have at least character within the str_input
![[UNCUT]"ตุลามรณะ"ย้อนฟังคำทำนาย หมอดูทุกสาย ดวงเมืองร้อน (หมอปลาย ฟองสนาน อ.โอเล่)I คนดังนั่งเคลียร์](http://i.ytimg.com/vi/lE5az5VvT9M/mqdefault.jpg)
Tried this on a python dev team. Led to team members committing less often, as each commit kicked off a small battle with the autoformatter and linter. Refactor code tickets were the worst. You comment out big chunks of functionality temporarily to keep things working through the transition, then the formatter removes all the orphaned variables/imports, which can take hours to reproduce. The sweet spot in my opinion is to enforce lint before merging a PR into master.
Great to hear your experience, this is definitely something to be weary of. It sounds like you may have had your linter settings turned up too high, I would recommend setting it to only catch errors that you definitely don't want committed. But in any case there is definitely friction introduced by the process. If the friction is too much as a pre-commit hook, you can also specify the scripts to run at pre-push time so devs can work freely on their local copies without any friction.
+1, it's really better to put that in CI and enforce a good branching model with branch protection, status checks and code reviews on merge.
Not a fan of adding dependencies, having to have the whole environment setup just to make a quick change, etc.
@@misterGaseg Is this what happens if one doesn't do any testing with the changes one has done before commiting? It seems that with CI developers may tend to leave all testing/verification up to the automated parts of the process...
@@benhetland576 The workflow I prefer is to add unit tests that exercise each new code path as I go. But a commit is not a merge. Commits should be free and painless, with very few exceptions (large files comes to mind as an exception). Sometimes I commit just because I need to start a ticket over, and want to save everything in case some of it winds up being useful later. I don't want to test/lint changes that aren't destined for master.
@@bhc1892 Wouldn’t a prepush be useful then?
I usually use vscode with an auto-formatting option on save. but running tests on every commit sounds interesting. as always, thanks for the video
Doing that for small projects is ok, but imagine a large project where running all the tests (e2e, unit, integration, etc) take about an hour or more
Unless there is way to only run tests related to the files that were changed
Also, sometimes during a work on a development branch you may break your tests, but you still want to commit your code
@@jedabero nx workspaces are great for exactly what you described
Okay that’s nice and all, but nothing beats filev1, filev2, filev3, filev3_final, filev3_final2
In my experience git hooks don't really work in practice since you can't enforce them properly. You need to nag each individual developer to install them every time they clone a new repository.
What works much better is having a format check in CI and forbid merges unless all tests pass. Also, this has additional benefit of allowing developers to work at their own pace and only tidy up code when they are ready to merge.
I double that. The only way git hooks are useful is either you are working on a project alone or on a very small team, otherwise, it quickly gets out of hand. Getting every developer setting up hooks every time they move to a new project/repo or change the environment they work on is just not happening in Enterprise. As much as GitHooks are promising, as long as they are not part of the source code of the repo they not going to work for bigger teams.
The yaml config is part of the source code of the repo, so it's the same amount of work as telling devs to "pip install -r requirements.txt". Just add "pre-commit install" after that in their setup instructions. In fact, you could even make it auto-install when they "pip install -e ." the project by adding it as a dev install hook in the setup.py, or by telling them to "git clone --template" the project from a template that has the hooks installed. It seems like the real issue most people have is with the friction involved at commit time, specifically by a linter or formatter, not with the difficulty of setting it up. I would argue that there are plenty of commit hooks that _really_ should be done before committing, like checking that no large files, private keys, or AWS credentials are committed, or checking that no syntax errors in yaml, json, or Python code are committed. As for code formatting and linting, maybe moving those to pre-push hooks would lessen the burden. I find that littering the git tree with "fix lint warning, code format" commits makes git history/blame much less useful and catching those things at least before a push happens so commits can be amended or squashed helps a lot. Of course, you should still be using your CI hooks as well, pre-commit is by no means a substitute for that.
@@mCoding The way our business unit operates is that we do have a lot of developers, and they are often shuffled into different 5-20 man teams, and assigned to different projects, plus devs are all over the world now, but in the same time we only have single DevOps team supporting all the projects with DevOps stuff. My DevOps team did try to enforce git-hooks prior to that, it was even less setup than what you showcase in the video. We had our own scripts folder for various hooks sitting in the repo, and all developers needed to do after cloning for the 1st time was to update git config to point to our hooks folder instead of the default .git/hook folder. There was no need to install any additional modules, nor to do any additional initializations within the repo.
We had to constantly remind and nag developers to enable usage of hooks, after a year of that we gave up.
In regards to all the benefits of running pre-commit and other hooks - I am 100% on board and agree with you. But from experience within my company - if we have to rely on our developer's continuous mindfulness to enable some of our DevOps functionality, it's most likely not going to be 100% reliable.
@@konstantinzavgorodniy3098 Git hooks are a convenience for the developer and to encourage cleaner commit history. As mentioned, they do not substitute for CI jobs which also run these commands and prevent any merges on the remote if there are failures.
@@kevinkalanda9946 All CI-CD jobs are done on post factum basis, and a big chunk of that effort could be eliminated by ability to enforce git hook usage through the repo/version control itself.
Nice. I normally use pre-commit with shed, but I probably need that yaml linter you showed too.
Two of my favorite Python TH-camrs just merged (pun intended). Mr. Anthonks ftw!
What happened?
@@kokop1107 TH-camr/twitch streamer AnthonyWritesCode is one of the maintainers on Pre-Commit and Pre-Commit-CI tools on GH
On point, no BS, fast and understadable. Thank you. Subscribed!
Awesome, thank you!
Dang! I wish I saw this video sooner! I could've used the pre-push hooks to prevent myself pushing non-rebased fixup commits.
Incredible video as usual. Just a note: there is an erroneous colon on line 28 of the yaml file, in the .src argument :) Keep up the good work, always looking forward to your videos!
Thanks! The colon is not erroneous, paths are colon separated (a common linux convention for variables like $PATH). So this variable says current dir and src dir (I copied it from either pytest or mypy's config 😉)
@@mCoding Huh, that's weird, the check-yaml test fails on that line. Thanks for the heads-up in any case :)
Very weird. I just double checked to be sure and the pre-commit did not fail the yaml check on my machine. Perhaps a copy paste error? That should be line 30, not 28.
@@mCoding Yeah it was 28 on my machine because I left out the comments. I appreciate you checking it again, the exact message I get is:
`while scanning a plain scalar
in ".pre-commit-config.yaml", line 28, column 12
found unexpected ':'
in ".pre-commit-config.yaml", line 28, column 39`
I'm sure I'll figure it out
Oh and the actual line is
`args: [--application-directories=.:src/foo, --py38-plus]`
Thanks this saved me a lot of time
I like it when there are fixable errors that they're fixed during precommit
I would rather run this things once before a PR/MR is merged than with every single commit.
You absolutely can! Pre-commit allows you to pick the stage to run each script at, so you can just put them all to pre-push in your config and voila!
Sounds like something I want to run every month or so, but once I learn from the several runs, I just don't see the point and I have to uninstall the hook. That is probably trivial, of course. But, sounds great for security - that is something you want to really check, especially on very sensitive code.
my goto if i have a bad commit in my remote branch: git checkout HEAD~1 && git push -f origin HEAD: git reset --hard origin/
👀 git push -f
@@mCoding in war, love and git, everything is allowed.
Tried this in the past, but some checks really add a great deal of friction. Maybe I need to break up my projects more or commit less often, idk.
Totally understandable, but rather than committing less often, here are some things you can do to lessen the friction. (1) run pre-commit as a pre-push hook rather than pre-commit hook (2) don't install pre-commit as a hook at all (or "pre-commit uninstall") and just manually run "pre-commit run" when you want to run the checks (3) edit your linter/mypy config to only catch serious errors that you would not be okay with committing
Would love to see a deeper presentation of this.
Great explanation, thank you!
Also, I have question regarding, how to get almost *close to pycharm* behaviour in *vscode* _(using linters, formatters, language server)_
Please recommend, or may be make a video about that?
Very useful info! Thanks for sharing
You're welcome!
Thanks, very informative video
Thanks Im adding this to my projects
Excellent and very concise video. Consider me a new subscriber.
Glad to have you!
That will be handy.
Thank you!
Great content, like always. But I wouldnt use pre commit auto formatter, because of two reasons:
1. It may mess with the logic/what the code tries to achieve
2. Because of #1, I need to double check after auto-formatting
If a new commit is not expensive in your organization, I’d fix these issues with isort-flake8-mypy tests with new/different commits and with more control. Because the new changes will/should only be related to formatting/typing and should not change the logic.
The black auto formatter should not mess with the code's logic, since it compares the ASTs before and after formatting to make sure the code does the same thing it did before. This could be an issue with auto formatters for other languages, though
Yeah black checks that the AST's of the before and after are identical (1 or 2 very tame exceptions to this), so unless you are doing source code introspection there's not really any danger. However, pre-commit can also just be used as a pre-push tool (installed as a pre-push hook) or a manual tool. Even if it was manual it still makes it easy to run all your checks in one command.
Thanks for the explanations! Great help.
Hes James, nice Video. One thing to look out for with mypy and pre-commit is that pre-commit by default will only run mypy on the changed files. This is not ideal as your changes in one file will often affect code in other files. Moreover, since pre-commit runs each hook in it's own venv, mypy can't properly typecheck your code when you use external dependencies. These issues have bitten me once or twice in the past.
There's a better way to configure the hook, but it’s a bit more complicated. There's a good blog post about it by Jared Khan, I recommend looking it up some time.
Thanks for pointing this out! Feel free to post a link to it.
black cannot change the meaning of code. it's like the second thing they explain in the docs
could you go in depth into super init?
what if a dev does not run the command pre-commit install to generate hooks on local
i am titling more towards CI to run the formatter on build system
You should have shown the generated pre-commit file
The source code is available in the github link the description if you are interested!
But this tool requires the git inside the project(inside the container if it is used). It's not good enough for everyone, especially for docker users
We have GitHub blocked in our office laptops, so we can not really use this tool...
Pre-commit doesn't require GitHub, so your organization can host all the pre-commit hooks they want to pre-approve and host them on any infrastructure they want. Of course, this requires getting approval from within your org.
To me, for "pre-commit" hook to MODIFY files is the worst possible design. I could accept the pre-commit hook doing some checks, or running some reporting, but modify files?! Are you kidding me? I'd flip out.
Haha I understand your issue. Indeed, very few pre-commit hooks actually modify your files. Black/other formatters and end of line fixer are the only ones I know that I would actually use. I'm constantly using the autoformat option in my IDE so perhaps I'm too trusting of black, but it does verify the AST of your code is identical to the one it started with and I've never had an issue with it. I'd understand if you only wanted no-modification hooks to run though.
nice
I tried using pre-commit hooks to format and test my code automatically, but there are two things bothering me : showing a "commit error" simply because your code had an extra line in a random file irks me, I wish there was a simple "hey I formatted your code, check it again". Second thing is time : on enterprise code, tests often take several minutes to execute ; waiting this long to commit disrupts the workflow, and makes me (and any contributor) commit less often, which I don't want. Does anyone have this experience, and maybe tips to improve it ?
The error is saying "hey I formatted your file, check it again". It has to be an error because the commit has to not happen. That said I wouldn't mind an option to commit automatically after formatting, though I get that many people wouldn't want that.
pre-commit only checks the files that were staged for committing, so it should be really quick.
Also, you (intentionally) don't have a unit test hook in pre-commit and you should run the tests independently, probably less often (e.g. after several commits & before push)
Import point I forgot to mention in the video! Pre-commit is not the place to run your tests. If you look at the list of pre-commit hooks, you'll notice that there aren't any for running tests. This is precisely for the reason you mentioned, tests might take a long time. You should still run your tests manually before committing, or just let your CI run them after committing. As for whether it should call a failed commit an "error" vs just tell you "files were changed by pre-commit hooks, commit again after inspecting", that's a great thing to suggest on their github issues page!
How do you remove the git hooks tell me)
You can remove the git hooks with pre-commit uninstall, or by deleting the file from your .git/hooks folder
@@mCoding and add a -n arg to a commit to not run hooks for that specific action.
A hacker give this code to you
def password(str_input, str_key): -> int
Password_num = 0
str_key_len=len(str_key)
str_input_len=len(str_input)
for i in range(str_input_len):
Password_num += str_key.index(str_input[i])*str_key_len**(str_input_len-i-1)
return Password_num
This function can take a string and output the password needed to protect the string
Puzzle: Make a inverted function where argument are password and str_key
Input:
Line 1: password
Line 2: str_key
Output: A string
Constrains: str_key cannot have duplicate letter and must have at least character within the str_input
2:56 > _"modifications were made after files were staged [i.e. to work tree]. so, need to git add these."_
git sucks.
HELP
Not a fan of this, seems like a pain to set up and maintain.
pre-commit vs husky (node.js)? It looks like neither of them is actually meant of languages other than what they're written in
This is correct. I have husky in all of my node templates and pre-commit in my python templates.