Add pre-commit hook for TheAlgorithms/Python

[dhruvmanila]

As mentioned in #2509 (comment)

Describe your change:

• Add an algorithm?

Checklist:

• I have read CONTRIBUTING.md.
• This pull request is all my own work -- I have not plagiarized.
• I know that pull requests will not be merged if they fail the automated tests.
• This PR only changes one algorithm file. To ease review, please open separate PRs for separate algorithms.
• All filenames are in all lowercase characters with no spaces or dashes.
• If this pull request resolves one or more open issues then the commit message contains Fixes: #{$ISSUE_NO}.

[dhruvmanila]

There are a bunch of files that will be fixed if I run this.
Should I open another PR for those fixes?

Also, take a look at https://github.com/asottile/reorder_python_imports
This seems to be a better alternative to isort in terms of being a public repository.

[dhruvmanila]

We also need to inform people to enable it in their local copy by:

$ pip install pre-commit
$ pre-commit install

Also, there are badges for pre-commit and black.
Shall I add them?

[cclauss]

This is awesome! How long does it take to run on your machine? (If it is not fast, contributors will turn it off.) Please add the badges to README.md in a separate follow-up PR. Please fix the issues in a separate PR that we should land before this one.

[dhruvmanila]

By default, pre-commit runs only on the new files or modified files. We can change it to run on all the files all the time but that would be inefficient.

Also, the environment is installed only when it is run the first time and when the rev is changed (versions are changed).

[cclauss]

OK. Yes now I understand. After this lands, we can add something like this to pre-commit run --all-files https://github.com/cclauss/itinerant-tester/blob/master/.github/workflows/pre-commit.yml

[dhruvmanila]

I don't think so we need a workflow file for this as all this is done locally. The workflow file is run after the commit has been pushed and mainly used to check if every test passed. It's mostly just for the visuals as far as I can understand.

[dhruvmanila]

TODO:

• Separate PR for all the fixes BEFORE this is merged
• Follow-up PR for adding badges to README.md
• Follow-up PR for a GitHub Action to pre-commit run --all-files

Is that correct?

[cclauss]

A fraction of all contributors will install and run pre-commit. The workflow would check the work of the contributors who do not install the tool and it will allow us to remove all that complexity from .travis.yml which should speed up our test runs.

[dhruvmanila]

Got it!

[cclauss]

Do you have rights to click the Squash and merge button above?

[dhruvmanila]

Missed it! But I don't think so as that will be available only to members and owners.

Oh, it's still not merged then, no I don't have the rights.

[cclauss]

OK. We need the fix PR before we merge.

Also I added to the GitHub Action to your task list above.

[dhruvmanila]

@cclauss DON'T MERGE THIS YET.
We need to ignore some files which require whitespace and newlines.

[dhruvmanila]

@cclauss Please take a look at the changes:
EOF newline will be checked only for Python files and ignoring whitespace for that specific file as it requires it in pytest

[dhruvmanila]

I can see the merge option on the app but not on the browser.

[cclauss]

THANKS!!

[DmytroLitvinov]

Hi @dhruvmanila ,

I guess we should warn peope about pre-commit at CONTRIBUTING.md file.

[cclauss]

Let's not force the complexities of a local install of pre-commit on all contributors. Some folks are pretty junior and adding pre-commit to their machines might turn them off from contributing. The README should however show them how to find and solve issues when the GitHub Action does not love their contribution.

[dhruvmanila]

Maybe we should put it in Travis_CI_tests_are_failing and change the name of the file to tests_are_failing and put the instructions as to how to find the error for all the automatic tests we're performing in that file.

The other idea would be to separate out the files for each test but having a lot of files doesn't seem attractive.