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

Refactor #4

Open
wants to merge 25 commits into
base: main
Choose a base branch
from
Open

Refactor #4

wants to merge 25 commits into from

Conversation

MawKKe
Copy link
Owner

@MawKKe MawKKe commented Jul 18, 2024

Rewrite the app for exercise:

  • Better, explicit data model using dataclasses
  • 100% type annotation coverage + strict mypy checks (in CI)
  • Near 100% testing coverage
  • Simplify code structure; remove unnessary functions
  • Implement the main app parallelism with asyncio (as an experiment)
  • Make CI the automated code quality guardian
  • Use pre-commit locally to ensure commits are consistent

MawKKe added 25 commits July 16, 2024 11:51
Module is tested, but not yet in use in the main code
Also disable coverage reporting for if __name__ = '__main__' blocks
because they usually contain nothing important anyway and just
decrease the coverage % unnecessarily
Both new and old implementations now pass the strict tests;
the old code needed some ugly typing.cast() calls to suppress
two valid but useless complaints.
Not sure if this is a good idea. There might be a chicken-vs-egg
situation arising from the pip-tools being installed inside the venv
we are constructing. We shall see...
It didn't work previously because the GH action script
installed dependencies from requirements-dev.txt, but
the _package itself_ was never installed; this meant
that the project.scripts were never placed into path
and thus failed the pytest case
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.

1 participant