-
Notifications
You must be signed in to change notification settings - Fork 31
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
Set up chrome-types for upcoming schema changes. #67
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #67 +/- ##
==========================================
+ Coverage 43.96% 44.20% +0.23%
==========================================
Files 21 21
Lines 4035 4052 +17
Branches 265 270 +5
==========================================
+ Hits 1774 1791 +17
Misses 2261 2261 ☔ 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.
Left a couple of thoughts, but otherwise this looks great. Thanks so much for the time on this and for adding a test.
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.
Oh right, I left these pending and didn't realize github requires one to "Submit review" to post them. I believe I've got all the changes in that were requested now.
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.
Thanks for the work making these changes and your patience! LGTM.
There's an upcoming schema change on the Chromium side which will flip all asynchronous returns to be defined using the "returns_async" field, instead of only for Promise-supporting functions. Previously this repo used the presence of the "returns_async" field as an indicator of promise support, but with the upcoming schema changes we instead need to look for the "does_not_support_promises" property on the "returns_async" field.
This pull request adds in support for looking for this new property and only returning the callback signature if it is there, while also remaining backwards compatible with the older form so the change can happen cleanly.