-
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
test: improvements and fixes #262
test: improvements and fixes #262
Conversation
Unit type implementation served no purpose.
Discussed and no action required for now.
Easier to see exactly what is being charged for in any specific call/test.
Very few functions left after refactor.
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.
Nice clean commits.
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.
Looks clean overall, left a few comments for my concerns.
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!
Codecov ReportAttention: Patch coverage is
@@ Coverage Diff @@
## chungquantin/extension-tests #262 +/- ##
================================================================
+ Coverage 41.83% 45.24% +3.40%
================================================================
Files 39 40 +1
Lines 3468 3698 +230
Branches 3468 3698 +230
================================================================
+ Hits 1451 1673 +222
- Misses 1969 1971 +2
- Partials 48 54 +6
|
Some improvements and fixes whilst reviewing #240.
See commit log for further details.
PS - sincere apologies about the last two commits, which were triggered by the fact that this PR tree has not been rebased on main. Rather than reformatting everything, to reformat again in the coming days, I think it made sense to bring in the nightly formatting and apply the fixes accordingly. We would have to tackle it imminently in any case. You should be able to select a range of commits when reviewing to exclude the last two.