-
Notifications
You must be signed in to change notification settings - Fork 0
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
Feature: add meta on get hooks #38
base: master
Are you sure you want to change the base?
Conversation
JuraJuki
commented
Aug 25, 2020
- references Add meta in getAll requests #16
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 pretty good, it will be a helpful addition!
Some things to keep in mind:
- these would be awesome to cover with
- there is a change some of these getOperationMeta calls are incomplete as they're missing id and request method (I think the compiler does not warn about it because they're optional)
@renatoruk updated get tests with meta expects. Added additional params for getOperationMeta. If ID is passed at useGet or useGetControlled the meta returned is undefined. Checked the checkboxes |
Looks good!
|
Codecov Report
@@ Coverage Diff @@
## master #38 +/- ##
==========================================
+ Coverage 85.41% 85.57% +0.16%
==========================================
Files 49 49
Lines 1049 1061 +12
Branches 161 161
==========================================
+ Hits 896 908 +12
Misses 151 151
Partials 2 2
Continue to review full report at Codecov.
|
It is a bug, it should return meta :C |
Did you have the time to work on this? As you said, it's prerequisite for #41 |
I did not sadly, should be having a couple of hours in the next few days so I will take a lot. Or perhaps you wish to take a look? |
Can you make the tests actually fail (prepare them with the real expected result) so it's easier to debug on what is needed? |