-
Notifications
You must be signed in to change notification settings - Fork 252
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
refactor(widget): tidy up and comment code a bit #4263
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4263 +/- ##
==========================================
+ Coverage 84.92% 84.95% +0.03%
==========================================
Files 274 274
Lines 29805 29798 -7
==========================================
+ Hits 25311 25314 +3
+ Misses 4494 4484 -10 ☔ View full report in Codecov by Sentry. |
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 good, thanks!
// messages. Refer to the `widget-api-poc` for implementation notes. | ||
error!("Failed to parse incoming message: {e}"); | ||
Err(error) => { | ||
error!("couldn't deserialized incoming widget message: {error}"); |
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.
error!("Couldn't deserialize incoming widget message: {error}");
6515803
to
4130157
Compare
I've been exploring the widget API so as to understand how it works, just for the purpose of review. I couldn't help but tweak things here and there; I think there's still a lot to be done before this code is understandable, but here's a small improvement over the status quo already.
cc @toger5