search

microsoft / vscode-python

Python extension for Visual Studio Code
https://aka.ms/pvsc-marketplace
MIT License
4.31k stars 1.18k forks source link

Add githooks #9363

Closed DonJayamanne closed 3 years ago

DonJayamanne commented 4 years ago

Creating an issue to address @kimadeline suggestions.

https://github.com/microsoft/vscode-python/pull/9350#issuecomment-570288111

What will be included in the CI changes, CI failing if prettier needs to reformat code? prettier doing the reformatting directly as one of the first steps?

If option 1 (CI failure because code needs to be reformatted), adding pre-commit hooks to prevent incorrectly formatted code to be committed would cause less overhead in the long run.

Note: I still use the pre-commit hooks in our extension. husky is part of the package.json & .huskyrc.json is excluded from git in .gitignore for this reason (so others could continue using it).

kimadeline commented 4 years ago

Dug out the documented reason why the hooks where removed ⛏ : https://github.com/microsoft/vscode-python/issues/2012#issuecomment-405372620

And I think we can skip doing pre-commit hooks because we have something called extensions. 😁 Pre-commit hooks are great as a standardized way to help make sure people meet some requirement before sending code to CI. But since we have standardized our tooling all the way up to the editor, we can actually just standardize on VS Code extensions to do the formatting for us and then let CI enforce that requirement as @d3r3kk suggested.

DonJayamanne commented 4 years ago

Closing as it was discussed, and the explanation was good enough not to warrant hooks.

kimadeline commented 4 years ago

It was more than a year ago though, and until your last few PRs formatting was all over the place and not standardized whatsoever.

I think it would be fair to revisit this decision, or at least reopen this issue so that we can discuss it, what do you think? Worst case it gets a big no and we bury it until the team gets a new member 🙃