-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
Move notification APIFormat #13783
Move notification APIFormat #13783
Conversation
Codecov Report
@@ Coverage Diff @@
## master #13783 +/- ##
==========================================
- Coverage 42.23% 42.22% -0.01%
==========================================
Files 699 700 +1
Lines 76959 76959
==========================================
- Hits 32500 32496 -4
- Misses 39094 39097 +3
- Partials 5365 5366 +1
Continue to review full report at Codecov.
|
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
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.
I'm still not certain about whether convert is the nicest design - I guess it's one way of coming up with an exportable set of models even if we don't use them internally.
However, this PR makes notification consistent with the rest of the library, therefore LGTM.
as title