Это видео недоступно.
Сожалеем об этом.

The easy way to keep your repos tidy.

Поделиться
HTML-код
  • Опубликовано: 15 авг 2024
  • Catch simple issues before code review using pre-commit.
    Pre-commit uses git hooks in order to catch your little typos before they hit the git tree. Automatically format your code, run a linter, type check, check config files, and more.
    ― mCoding with James Murphy (mcoding.io)
    Source code: github.com/mCo...
    All videos source code: github.com/mCo...
    pre-commit: pre-commit.com/
    Popular hooks: pre-commit.com...
    SUPPORT ME ⭐
    ---------------------------------------------------
    Patreon: / mcoding
    Paypal: www.paypal.com...
    Other donations: mcoding.io/donate
    Top patrons and donors: Jameson, Laura M, Dragos C, Vahnekie, John Martin, Casey G, Pieter G, Krisztian M, Sigmanificient
    BE ACTIVE IN MY COMMUNITY 😄
    ---------------------------------------------------
    Discord: / discord
    Github: github.com/mCo...
    Reddit: / mcoding
    Facebook: / james.mcoding
    CHAPTERS
    ---------------------------------------------------
    0:00 Intro
    0:42 Git hooks
    1:33 Install and use pre-commit
    3:46 Favorite pre-commit hooks
    4:39 After a config change
    5:00 Is this safe?

Комментарии • 86

  • @bhc1892
    @bhc1892 2 года назад +141

    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.

    • @mCoding
      @mCoding  2 года назад +47

      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.

    • @misterGaseg
      @misterGaseg 2 года назад +7

      +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.

    • @benhetland576
      @benhetland576 2 года назад

      @@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...

    • @bhc1892
      @bhc1892 2 года назад +1

      ​@@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.

    • @xllvr
      @xllvr 2 года назад +1

      @@bhc1892 Wouldn’t a prepush be useful then?

  • @DmitriyVasil
    @DmitriyVasil 2 года назад +60

    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

    • @jedabero
      @jedabero 2 года назад +4

      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

    • @nadavgolden
      @nadavgolden 2 года назад +7

      Also, sometimes during a work on a development branch you may break your tests, but you still want to commit your code

    • @edmondoflynn2064
      @edmondoflynn2064 2 года назад +1

      @@jedabero nx workspaces are great for exactly what you described

  • @Jakub1989YTb
    @Jakub1989YTb 2 года назад +16

    Two of my favorite Python RUclipsrs just merged (pun intended). Mr. Anthonks ftw!

    • @kokop1107
      @kokop1107 2 года назад

      What happened?

    • @Ash-qp2yw
      @Ash-qp2yw 2 года назад +1

      @@kokop1107 RUclipsr/twitch streamer AnthonyWritesCode is one of the maintainers on Pre-Commit and Pre-Commit-CI tools on GH

  • @pedrovalois3580
    @pedrovalois3580 2 года назад +6

    Nice. I normally use pre-commit with shed, but I probably need that yaml linter you showed too.

  • @mushrafaltaf
    @mushrafaltaf 2 года назад +1

    On point, no BS, fast and understadable. Thank you. Subscribed!

    • @mCoding
      @mCoding  2 года назад +1

      Awesome, thank you!

  • @Rizhiy13
    @Rizhiy13 2 года назад +26

    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.

    • @konstantinzavgorodniy3098
      @konstantinzavgorodniy3098 2 года назад +2

      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.

    • @mCoding
      @mCoding  2 года назад +14

      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.

    • @konstantinzavgorodniy3098
      @konstantinzavgorodniy3098 2 года назад +1

      ​@@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.

    • @kevinkalanda9946
      @kevinkalanda9946 2 года назад +1

      @@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.

    • @konstantinzavgorodniy3098
      @konstantinzavgorodniy3098 2 года назад

      @@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.

  • @liesdamnlies3372
    @liesdamnlies3372 2 года назад +16

    Okay that’s nice and all, but nothing beats filev1, filev2, filev3, filev3_final, filev3_final2

  • @johnrussell6971
    @johnrussell6971 Год назад

    Dang! I wish I saw this video sooner! I could've used the pre-push hooks to prevent myself pushing non-rebased fixup commits.

  • @falxie_
    @falxie_ 2 года назад

    I like it when there are fixable errors that they're fixed during precommit

  • @barrientoscardenaslinofern4717
    @barrientoscardenaslinofern4717 2 года назад +1

    Thanks this saved me a lot of time

  • @jonathanw8004
    @jonathanw8004 2 года назад +9

    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!

    • @mCoding
      @mCoding  2 года назад +3

      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 😉)

    • @jonathanw8004
      @jonathanw8004 2 года назад

      @@mCoding Huh, that's weird, the check-yaml test fails on that line. Thanks for the heads-up in any case :)

    • @mCoding
      @mCoding  2 года назад +2

      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.

    • @jonathanw8004
      @jonathanw8004 2 года назад +1

      @@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

    • @jonathanw8004
      @jonathanw8004 2 года назад +2

      Oh and the actual line is
      `args: [--application-directories=.:src/foo, --py38-plus]`

  • @Kralnor
    @Kralnor 2 месяца назад

    Excellent and very concise video. Consider me a new subscriber.

    • @mCoding
      @mCoding  2 месяца назад

      Glad to have you!

  • @PythonisLove
    @PythonisLove 2 года назад +2

    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?

  • @computersciencestuff3405
    @computersciencestuff3405 2 года назад

    Great explanation, thank you!

  • @georgesmith9178
    @georgesmith9178 Год назад

    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.

  • @Filaxsan
    @Filaxsan 2 года назад

    Very useful info! Thanks for sharing

    • @mCoding
      @mCoding  2 года назад

      You're welcome!

  • @jacobwalters9660
    @jacobwalters9660 2 года назад

    Thanks Im adding this to my projects

  • @objectobject5889
    @objectobject5889 2 года назад +3

    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/

    • @mCoding
      @mCoding  2 года назад +13

      👀 git push -f

    • @mfrederikson
      @mfrederikson 2 года назад +6

      @@mCoding in war, love and git, everything is allowed.

  • @drooten
    @drooten 2 года назад

    Would love to see a deeper presentation of this.

  • @PythonisLove
    @PythonisLove 2 года назад

    Thanks, very informative video

  • @wewep6649
    @wewep6649 2 года назад

    That will be handy.
    Thank you!

  • @ManuelBTC21
    @ManuelBTC21 2 года назад +1

    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.

    • @mCoding
      @mCoding  2 года назад +4

      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

  • @somebodystealsmyname
    @somebodystealsmyname 2 года назад +3

    I would rather run this things once before a PR/MR is merged than with every single commit.

    • @mCoding
      @mCoding  2 года назад +3

      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!

  • @SemihTekten
    @SemihTekten 2 года назад

    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.

    • @danhorus
      @danhorus 2 года назад +4

      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

    • @mCoding
      @mCoding  2 года назад +1

      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.

    • @SemihTekten
      @SemihTekten 2 года назад

      Thanks for the explanations! Great help.

  • @rashidnizamuddin3700
    @rashidnizamuddin3700 11 месяцев назад

    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

  • @BeenOrange
    @BeenOrange 2 года назад

    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.

    • @mCoding
      @mCoding  2 года назад

      Thanks for pointing this out! Feel free to post a link to it.

  • @fredericobsantos
    @fredericobsantos 2 года назад

    could you go in depth into super init?

  • @Lunolux
    @Lunolux 2 года назад

    nice

  • @sakost
    @sakost 2 года назад +1

    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

  • @dqalombardi
    @dqalombardi 2 года назад

    black cannot change the meaning of code. it's like the second thing they explain in the docs

  • @tinahalder8416
    @tinahalder8416 4 месяца назад

    We have GitHub blocked in our office laptops, so we can not really use this tool...

    • @mCoding
      @mCoding  4 месяца назад

      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.

  • @gus3000spam
    @gus3000spam 2 года назад +1

    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 ?

    • @cool_scatter
      @cool_scatter 2 года назад +2

      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.

    • @Bryft
      @Bryft 2 года назад

      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)

    • @mCoding
      @mCoding  2 года назад +3

      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!

  • @roor5303
    @roor5303 2 года назад

    You should have shown the generated pre-commit file

    • @mCoding
      @mCoding  2 года назад

      The source code is available in the github link the description if you are interested!

  • @TheInimicus
    @TheInimicus 2 года назад

    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.

    • @mCoding
      @mCoding  2 года назад

      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.

  • @Agrover112
    @Agrover112 2 года назад

    How do you remove the git hooks tell me)

    • @mCoding
      @mCoding  2 года назад

      You can remove the git hooks with pre-commit uninstall, or by deleting the file from your .git/hooks folder

    • @morpheus636
      @morpheus636 2 года назад

      @@mCoding and add a -n arg to a commit to not run hooks for that specific action.

  • @26-dimesional_Cube
    @26-dimesional_Cube 2 года назад

    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

  • @markcuello5
    @markcuello5 Год назад

    HELP

  • @yash1152
    @yash1152 4 месяца назад

    2:56 > _"modifications were made after files were staged [i.e. to work tree]. so, need to git add these."_
    git sucks.

  • @nO_d3N1AL
    @nO_d3N1AL 2 года назад

    Not a fan of this, seems like a pain to set up and maintain.

  • @AssemblyWizard
    @AssemblyWizard 2 года назад

    pre-commit vs husky (node.js)? It looks like neither of them is actually meant of languages other than what they're written in

    • @morpheus636
      @morpheus636 2 года назад +2

      This is correct. I have husky in all of my node templates and pre-commit in my python templates.