Icevision: [Discussion] Add `pre-commit` as development dependency

[partham16]

🚀 Feature

Is your feature request related to a problem? Please describe.
Continuing from discord, this issue is raised so that we can discuss and decide what to include in the pre-commit hooks

Describe the solution you'd like
For pre-commit there are multiple hooks we can add. Should the goal be to add all possible hooks (for the dev dependencies)?

Describe alternatives you've considered
Or should we just start small? Since the github actions seems to check for black, pytest and flake8 - these should be added in first?

Although pytest at every commit might be too much - can be left out at the discretion of the PR submitter.

[lgvaz]

Hi @partham16! Thanks for moving forward with this one!

As you suggested, let's start small, black and flake8 are the most important hooks.

Quick question, flake8 checks code syntax errors, but doesn't black also checks for that? (It cannot format code with syntax error)

[partham16]

AFAIK, black checks the syntax tree for formatting - so, that makes sense (btw have you seen an instance where black pointed out a syntax error? I either have pylance or pylint/flake8 enabled - so, even if it can - black probably doesn't even get a chance)

I guess flake8 or pylint is more excessive - like checking for unused-import, or even the fact that the import can happen at all etc.

[lgvaz]

I guess flake8 or pylint is more excessive - like checking for unused-import

Makes sense! Let's go with black + flake8 combo, there is no downside of adding flake8 anyways :smile: