Conversation
|
r? @domfarolino |
|
LGTM, though I'm wondering if we need to specify the side effects that happen in some of these other environments. |
|
On a first glance, I'm not seeing anything that obviously needs adjusting, but let me know if you disagree. |
|
@terinjokes @domfarolino can you sort out the CI failure? Seems like an image might need width/height attributes? |
domfarolino
left a comment
There was a problem hiding this comment.
Regarding tests, you have:
TBD but quite trivial
This PR LGTM, but do you plan on adding tests?
|
@domfarolino For tests, I'll make sure to add an idlharness test as soon as the IDL changes propagate into the wpt repo after this PR is merged. I don't think there's much else I can test - maybe a manual test that the messages show up? |
|
A manual test would be nice I think |
|
Tests are up at web-platform-tests/wpt#34361 |
|
This and the tests LGTM. GitHub is complaining about the branch not being up to date, so if you could fix that then I think we'd be good to go. |
7bb6921 to
334b6f3
Compare
|
Done! |
|
Great, feel free to merge the tests whenever and I'll go ahead and merge this. |
|
Merged the wpt pr, thank you |
Ref: tc39/proposal-shadowrealm#331
(See WHATWG Working Mode: Changes for more details.)
Preview | Diff