-
Notifications
You must be signed in to change notification settings - Fork 6
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
Remove handler #538
Remove handler #538
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #538 +/- ##
==========================================
- Coverage 91.51% 90.21% -1.31%
==========================================
Files 42 41 -1
Lines 1933 1829 -104
==========================================
- Hits 1769 1650 -119
- Misses 164 179 +15 ☔ View full report in Codecov by Sentry. |
b04c12c
to
d200a0c
Compare
9dba177
to
4c480cc
Compare
64bcaa2
to
81be95f
Compare
The handler has caused additional complexity with multiple levels of indirection. Remove the ABC and include the free functions in an "interface" module. The main app and cli tests are rewritten to look more like unit tests. This PR is to be followed up by further refactoring to replace the singleton class with cached functions and restructuring the modules. Fixes #434.
3794a34
to
533bea0
Compare
533bea0
to
6ae803e
Compare
Had a play locally, when I reload the environment it doesn't appear to import new changes. |
30774d2
to
de535ac
Compare
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.
LGTM!
Remove handler
The handler has caused additional complexity with multiple levels of
indirection.
Remove the ABC and include the free functions in an "interface" module.
The main app and cli tests are rewritten to look more like unit tests.
This PR is to be followed up by further refactoring to replace the
singleton class with cached functions and restructuring the modules.
Fixes #434.