Closed jeffsawatzky closed 1 month ago
Attention: Patch coverage is 96.96970%
with 2 lines
in your changes missing coverage. Please review.
Project coverage is 91.31%. Comparing base (
517f830
) to head (7f9b286
). Report is 7 commits behind head on develop.
Files with missing lines | Patch % | Lines |
---|---|---|
amazonorders/entity/order.py | 77.77% | 2 Missing :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
This is great, thank for taking the time to look at these issues. I had been ignoring them for now, since I disagreed with a lot of the things mypy
was saying were "issues", and didn't want to cause regressions diving in to them.
Just a heads up that I won't be able to fully dive in to reviewing this PR for a week or two, very busy right now, and I want to make sure I give it the time it deserves. Thank you in advance for the contribution!
Just a few comments. The primary blocker for me are the assert
additions, if you could raise proper exceptions instead. Otherwise the other comments should be trivial fixes, then I'm good to merge. Thanks!
I want to get the changes in develop
out ASAP, since Amazon has UI changes coming that are breaking the current implementation, and I want to cut a new release. I plan to merge this PR to develop
later tonight, then I will address the comments I left on it before pushing the changes in a release. Thanks for your contribution!
Description
Fixes the mypy issues.
Type of Change
Testing Done
A clear and concise description of the new tests added to validate the change as well as any manual testing done.
Checklist
make test
passes with no new errors or warningsmake check
to ensure no new errors or warningsmake docs
to ensure no new errors or warnings