-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
fix(types): include 'as' in IncludeThroughOptions definition #11624
fix(types): include 'as' in IncludeThroughOptions definition #11624
Conversation
Hello, thanks for the PR, I have a question in the issue though, please take a look :) |
Since this is currently undocumented, I don't think this should be merged as-is... @mrcrow85 would you be willing to update the docs with this fact as well? In fact, can you first create an sscce showing this thing working? The issue's OP does not have time, thanks! |
@papb Sure! I can do that |
Thanks for the work so far @mrcrow85, I have some requests before we can merge this. Since this feature seems a bit obscure to me, I think more has to be done:
What do you think? |
@mrcrow85 Not every feature is explained in the guides/tutorials, so I wouldn't say it is strictly necessary. Also, since I happen to be in the middle of rewriting the guides already, don't worry about it. |
Codecov Report
@@ Coverage Diff @@
## master #11624 +/- ##
=======================================
Coverage 96.26% 96.26%
=======================================
Files 94 94
Lines 9190 9190
=======================================
Hits 8847 8847
Misses 343 343
Continue to review full report at Codecov.
|
@papb Does the codecov report is ok? Seeing 21% less coverage really worries me. |
Ok, nevermind. It fixed itself 🤔 |
Thanks @mrcrow85, great work! |
Pull Request check-list
Please make sure to review and check all of these items:
npm run test
ornpm run test-DIALECT
pass with this change (including linting)?Description of change
This will add
as
property to theIncludeThroughOptions
definition.Closes #11185