-
Notifications
You must be signed in to change notification settings - Fork 4
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
Fix quality #183
Fix quality #183
Conversation
Will take a closer look later this morning, but I think it's failing because of the two errors being thrown in the
|
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.
Looks good overall. Few minor things:
- Pylint precommit (see comment)
- We probably don't want the .json and .fcsv files to be linted, which it looks like they were in this case.
The request for change is related to the linted fiducial files. Could probably just remove them from the PR and update the linters to ignore those files.
# hooks: | ||
# - id: flake8 | ||
|
||
# Pylint needs to be installed locally for precommit |
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.
Not opposed to not including the pylint in the pre-commit but just wondering if there was any particular reason for removing it?
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 couldn't get it to work properly -- precommit didn't seem to play nicely with my local installation of pylint. It wouldn't pick up the pylint_flask_sqlalchemy
plugin and ignored the # pylint: disable=...
directives I added, leading it to fail every time (similar to what happened in the GH action).
I think the |
Hmm okay, I'll play around with it a little more this morning and see if I can get that part working then. |
Proposed changes
Get all our quality checkers passing.
I couldn't get pylint to work properly in pre-commit, but it passes on my machine too.
Types of changes
What types of changes does your code introduce? Put an
x
in the boxes that applyChecklist
Put an
x
in the boxes that apply. You can also fill these out after creating the PR. If you are unsure about any of the choices, don't hesitate to ask!black
with the-l 79
flag.Notes
All PRs will undergo the unit testing before being reviewed. You may be requested to explain or make additional changes before the PR is accepted.
PR template was adopted from appium