Skip to content

hooks: pylint: fix pre-commit#4146

Closed
casperdcl wants to merge 1 commit into
treeverse:masterfrom
casperdcl:hooks-pylint
Closed

hooks: pylint: fix pre-commit#4146
casperdcl wants to merge 1 commit into
treeverse:masterfrom
casperdcl:hooks-pylint

Conversation

@casperdcl
Copy link
Copy Markdown
Contributor

@casperdcl casperdcl commented Jul 1, 2020

  • correctly define pylint pre-commit hook

.pre-commit-config.yaml states that the pylint hook is defined locally, yet .pre-commit-hooks.yaml fails to define it. As a result, attempting to commit will result in an error.

Defining the hook's language as system and asking devs to manually run pip install .[tests] goes against fundamental principles of pre-commit hooks. None of the other hooks require this.

In any case pylint itself defines https://github.com/PyCQA/pylint/blob/master/.pre-commit-hooks.yaml so we should just use theirs.

@casperdcl casperdcl requested review from efiop and skshetry July 1, 2020 17:16
@casperdcl casperdcl self-assigned this Jul 1, 2020
@casperdcl casperdcl added bug Did we break something? build testing Related to the tests and the testing infrastructure labels Jul 1, 2020
@casperdcl casperdcl removed their assignment Jul 1, 2020
@casperdcl
Copy link
Copy Markdown
Contributor Author

casperdcl commented Jul 1, 2020

@skshetry I guess you'll have more experience with this but I suspect we need something like --init-hook="sys.path.append($SOME_EXTERNAL_PYTHONPATH)" to fix the import errors.

@efiop
Copy link
Copy Markdown
Contributor

efiop commented Jul 15, 2020

#4145 (comment)

@efiop efiop closed this Jul 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Did we break something? testing Related to the tests and the testing infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

hooks: pylint: pre-commit failure

2 participants