Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🔧 MAINT: fix pre-commit #123

Merged
merged 4 commits into from
May 27, 2021

Conversation

hugovk
Copy link
Contributor

@hugovk hugovk commented May 11, 2021

The CI is failing, for example:

Error: The hook black requires pre-commit version 2.9.2 but version 1.17.0 is installed. Perhaps run pip install --upgrade pre-commit.

The first commit here updates the pre-commit pin to the latest 2.12.1. (Perhaps even unpin this?)


Next, when running pre-commit, it warns:

[WARNING] The 'rev' field of repo 'https://github.com/psf/black' appears to be a mutable reference (moving tag / branch). Mutable references are never updated after first install and are not supported. See https://pre-commit.com/#using-the-latest-version-for-a-repository for more details. Hint: pre-commit autoupdate often fixes this.

The second commit runs pre-commit autoupdate to update both Black and pre-commit-hooks.


Next, pre-commit-hooks errors due to the Flake8 hook having been moved:

Check JSON...........................................(no files to check)Skipped
Check Yaml...............................................................Passed
Fix End of Files.........................................................Passed
Trim Trailing Whitespace.................................................Passed
Flake8 (removed).........................................................Failed
- hook id: flake8
- exit code: 1

`flake8` has been removed -- use `flake8` from https://gitlab.com/pycqa/flake8

black................................................(no files to check)Skipped

The third commit updates the hooks so it uses Flake8 from its new home at https://github.com/PyCQA/flake8.


Finally, 4th commit: we don't need to explicitly install Black and Flake8 into the environment for them to be used by pre-commit. In fact, it's unnecessary because pre-commit manages its own dependencies.

@welcome
Copy link

welcome bot commented May 11, 2021

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out EBP's Code of Conduct and our Contributing Guide, as this will greatly help the review process.

Welcome to the EBP community! 🎉

@hugovk hugovk changed the title CI: fix pre-commit [TST] CI: fix pre-commit May 11, 2021
@hugovk hugovk changed the title [TST] CI: fix pre-commit [TST, FIX] CI: fix pre-commit May 11, 2021
@hugovk
Copy link
Contributor Author

hugovk commented May 11, 2021

Let me know if you'd like me to redo the commit messages with emoji!

https://github.com/executablebooks/.github/blob/master/CONTRIBUTING.md#commit-messages

Copy link
Contributor

@sappelhoff sappelhoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran into these problems as well and I think this is a good fix.

@choldgraf choldgraf changed the title [TST, FIX] CI: fix pre-commit 🔧 MAINT: fix pre-commit May 27, 2021
@choldgraf choldgraf merged commit fa0845b into executablebooks:master May 27, 2021
@welcome
Copy link

welcome bot commented May 27, 2021

Congrats on your first merged pull request in this project! 🎉
congrats

Thank you for contributing, we are very proud of you! ❤️

@choldgraf
Copy link
Member

ah I missed this PR, thanks very much @hugovk and thanks @sappelhoff for the ping which made me realize this PR was here :-)

@hugovk hugovk deleted the update-pre-commit branch May 28, 2021 06:31
@hugovk
Copy link
Contributor Author

hugovk commented May 28, 2021

You're welcome, thank you for this helpful tool (and all the others too)!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants