-
Notifications
You must be signed in to change notification settings - Fork 41
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
editoast: make intersection as a type #8897
Conversation
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## dev #8897 +/- ##
============================================
+ Coverage 37.11% 37.19% +0.08%
- Complexity 2211 2245 +34
============================================
Files 1260 1260
Lines 115145 115468 +323
Branches 3230 3267 +37
============================================
+ Hits 42737 42953 +216
- Misses 70475 70569 +94
- Partials 1933 1946 +13
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
cd98dc3
to
9f8b3a1
Compare
FWIW I think it's completely fine to use tuples in internal editoast APIs. I don't think it's a big deal if we need to convert them before sending them over HTTP. |
Well, to me, |
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 improvement:)
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, thanks for this PR
08a956c
to
1da8257
Compare
1da8257
to
65cf266
Compare
65cf266
to
95b0d70
Compare
Signed-off-by: Jean SIMARD <[email protected]>
95b0d70
to
dc827c6
Compare
Might help #8794 (comment). And in general, explicit typing is not necessarily bad, just more verbose.