-
-
Notifications
You must be signed in to change notification settings - Fork 6
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
GitHub Actions: Run pre-commit #74
Conversation
py/test/test_endpointer.py
Outdated
@@ -4,7 +4,6 @@ | |||
import unittest | |||
|
|||
import numpy as np | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a problem: when I run pre-commit run --all-files
on my machine, or isort directly, this change gets undone.
The problem is that soundswallower is an internal module, and therefore there's a space between third party import (numpy) and internal imports, but pre-commit-action is not recognizing that here, it's lumping soundswallower with the third party imports.
Maybe a pip install -e .
is required first?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK... Let's try that. There is also the idea of isort known-first-party
in the config but I like your idea more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose for just running isort, it's silly to install everything.
I was going to vote against known-first-party because I think isort ought to be able to figure it out, but for performance reasons I have changed my mind.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good now.
PR Goal?
Run the pre-commit tests in GitHub Actions to catch issues from users who do not have pre-commit installed locally.
Fixes?
Feedback sought?
Priority?
Tests added?
How to test?
Confidence?
Version change?