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

#392: Make pylint errors cause a CI failure #395

Merged
merged 4 commits into from
Jun 5, 2023

Conversation

cwschilly
Copy link
Contributor

Fixes: #392

@cwschilly cwschilly self-assigned this Jun 2, 2023
@cwschilly cwschilly linked an issue Jun 2, 2023 that may be closed by this pull request
@cwschilly
Copy link
Contributor Author

cwschilly commented Jun 2, 2023

There is still one pylint error:
src/lbaf/Applications/rank_object_enumerator.py:27:25: E1101: Instance of 'LoadReader' has no 'read' member (no-member)
@ppebay Is there a different function that has taken on the functionality of "read"?

@ppebay
Copy link
Contributor

ppebay commented Jun 5, 2023

There is still one pylint error: src/lbaf/Applications/rank_object_enumerator.py:27:25: E1101: Instance of 'LoadReader' has no 'read' member (no-member) @ppebay Is there a different function that has taken on the functionality of "read"?

Indeed @cwschilly, good catch, this is now entirely outdated as the LoadReader was deprecated and replaced by the VTDataReader with updated execution logic (especially re: new vt JSON scheme).

This stand-alone script is to be deprecated (as part of an ongoing discussion with @tlamonthezie in #386 ) so we should ignore this error until the script goes away entirely.

Copy link
Contributor

@ppebay ppebay left a comment

Choose a reason for hiding this comment

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

Looking good. Cf. my answer to your question @cwschilly

@cwschilly
Copy link
Contributor Author

Looking good. Cf. my answer to your question @cwschilly

Thank you @ppebay . Should I disable the pylint error in the code so that it ignores it, or should I keep it as it is (and we just ignore the failing test)?

@cwschilly cwschilly merged commit a476a26 into develop Jun 5, 2023
@cwschilly cwschilly deleted the 392-tox-pylint-fails-in-ci branch June 5, 2023 18:53
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.

tox: pylint fails in CI
2 participants