Black/Python Linting Github Workflow

[da-h]

Description

Following up on #877, this PR completes the linting-workflow by adding also Python linting using black.
Additionally, this PR:

• clarifies the name of the javascript linter (job name changed to Javascript Linter Check as opposed to Python Linter Check)
• adds a warning if the python linter failed, asking the user to run black py
• adds a note for both linter to the CONTRIBUTING.md file
• applies the changed that running black py results in to ensure a clean state for the current branch

How Has This Been Tested?

• Skimmed myself the changes of black py, which all seem valid for me.
• Succeeding Action Example (this very PR)
• Failing Action Example (this very PR without having run black py): https://github.com/da-h/visdom/actions/runs/3314943853

Screenshots (if appropriate):

A failing workflow will show up an additional CTA for the user:

Types of changes

• Github Workflow
• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Code refactor or cleanup (changes to existing code for improved readability or performance)

Checklist:

• I adapted the version number under py/visdom/VERSION according to Semantic Versioning
• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.

[JackUrb]

Awesome to have lint across the codebase! The only thing I'd also suggest is that we can even implement lint as a pre-commit hook (though that can be done in the future).

[da-h]

Good idea!
I'll add this feature tomorrow to this PR. 🔥

[da-h]

There are multiple ways to do a pre-commit hook depending on if black is installed on the machine:

• intrusive, i.e. should git commit fail if black is not installed? It seems it is already the case if npx is not installed.
• alternatively / in this PRs latest push: inform the user about the missing tool, but still allow the commit to be done. In this case the github action would throw an error anyways. See output below for an example.

> git commit -a -m "test"
⚠ Some of your tasks use `git add` command. Please remove it from the config since all modifications made by tasks will be automatically added to the git commit index.

→ No staged files match any configured task.
'black' could not be found. Install it using 'pip install black'
[python_formatter_black b344bcc] test
 1 file changed, 1 insertion(+), 2 deletions(-)

[JackUrb]

In Mephisto we tie to a specific version of black and let github's pre-commit handle the dependency inclusions. This also prevents thrash between different versions of the formatters.

[da-h]

Of course, it makes so much sense to fix the version! Didn't think of that. 😅

While visdom uses husky as a pre-commit hook manager, I do not see a native way to handle pip packages natively with it as well.

Possibly its better to change to the more all-in-one solution pre-commit for both, javascript and node?

Edit: Just saw that both can be used together as well. I am not sure yet what the pro/cons are of this approach.

[JackUrb]

I don't have strong preference. Whatever feels easiest to work with?

[da-h]

Since last push:

• added pre-commit with a configuration that also mimics behavior of husky + lint-staged
• fixed version of black (in github workflow and in pre-commit config)
• removed husky and lint-staged
• added a note to CONTRIBUTING.md about pre-commit

A change in behavior is that pre-commit does not git add automatically, compared to the previous setup of husky + lint-staged. (See for instance pre-commit/pre-commit#747 (comment))

[JackUrb]

Thanks @da-h for yet another improvement to the developer experience around here! Happy to see the inclusion of black.