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

Improve Code Health Infrastructure: Lint, Coverage, and Type Checking #22

Merged
merged 19 commits into from
Jan 6, 2021

Conversation

U8NWXD
Copy link
Member

@U8NWXD U8NWXD commented Oct 9, 2020

See #42 for how to use the features introduced here

@U8NWXD U8NWXD mentioned this pull request Jan 5, 2021
27 tasks
@U8NWXD U8NWXD changed the title Add pylint Improve Code Health Infrastructure: Lint, Coverage, and Type Checking Jan 6, 2021
@U8NWXD
Copy link
Member Author

U8NWXD commented Jan 6, 2021

@eagmon this PR is ready to go! Could you take a look?

requirements.txt Outdated Show resolved Hide resolved
Copy link
Member

@eagmon eagmon left a comment

Choose a reason for hiding this comment

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

Looks great! This will make it so much easier to keep the code health in check.

Maybe some of the text in #42 can be made into a doc? That would be easier long-term to reference than a GitHub issue

@U8NWXD U8NWXD merged commit 90c9c82 into master Jan 6, 2021
@U8NWXD U8NWXD deleted the pylint branch January 6, 2021 02:32
Copy link
Contributor

@1fish2 1fish2 left a comment

Choose a reason for hiding this comment

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

It's great to see these additional automated tests and the use of Python 3 type hints!

@@ -41,4 +41,5 @@
'pymongo',
'pytest',
'scipy',
'pylint',
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we remove 'pytest' and 'pylint' here, being dev tools not needed by clients of this library?

python-dateutil==2.8.1
scipy==1.5.4
six==1.15.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the six package just an indirect dependency or is it no longer needed now that we've dropped Python 2 (RIP)?

Idea: To aid future maintenance, we could group requirements.txt into sections containing

  • the direct dependencies -- anything in this group can be pruned out from here and setup.py if/when the code no longer imports it, like kafka
  • the dev tools -- not imported; not needed in setup.py; used in dev scripts
  • the frozen indirect dependencies

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.

None yet

3 participants