-
Notifications
You must be signed in to change notification settings - Fork 165
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
Update the WebIDL interfaces for the new optional dictionnary defaulting #1993
Update the WebIDL interfaces for the new optional dictionnary defaulting #1993
Conversation
…ing. This was done by running: ``` sed -i 's/optional\(.*\)Options options)/optional\1Options options = {})/' index.bs ```
This fixes #1989. |
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, so let's merge it after fixing errors.
We need the corresponding Deps (as mentionned in whatwg/webidl#758): plinss/widlparser#39 speced/bikeshed#1491 |
This seems a bit premature if none of the required infrastructure is available. I would propose that unless this happens in the next week or two, that this issue (and PR) get pushed to v2. Because it will be anyway if the infrastructure isn't available. |
Based on PR #2042, it looks like bikeshed has the necessary support for this now, so we're probably ready to land this. |
Hmm. There are other changes in play here. Take a look at the preview and look at each constructor. There was a table listing the parameters to the constructor, and columns "Type", "Nullable" and "Optional" used to have the type and x's or checkmarks to indicate if they're nullable or optional. The preview doesn't have these anymore. This seems like a regression in bikeshed? Missing the type is really unfortunate. |
Filed speced/bikeshed#1511 for this. |
We should probably wait on this since Tab says it's probably a bug. (See speced/bikeshed#1511 (comment)) |
Whatever the problem was it seems to be gone now when I build this branch locally. The tables have the right thing now. |
This was done by running:
Preview | Diff