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 to webidl2.js@22.0.0 #16864
Update to webidl2.js@22.0.0 #16864
Conversation
|
@jgraham @lukebjerring can one/both of you do a full run against this PR? |
I'm away - I think @foolip knows how to do this too? |
I've submitted https://chromium-review.googlesource.com/c/chromium/src/+/1630483 There were the presubmit warnings:
The trailing newline is easy to fix: w3c/webidl2.js#329 The single long line I don't know what to do about. @Hexcles WDYT? If we're only using the build webidl2.js I wonder if there's any use in us still pulling in the whole history of the repo. It still won't be possible to blame code that is actually run, since it's a single line. |
We also don't have the sourcemap working, which is potentially annoying when debugging things, due to the path rewriting… I don't know what the right solution there is. |
To be fair, we were only ever pulling in a single file. I think if anything there's a stronger argument now, because we could commit a non-minified built webidl2.js. |
https://test-results.appspot.com/data/layout_results/linux-rel/102964/webkit_layout_tests%20%28with%20patch%29/layout-test-results/results.html shows all idlharness.js tests now failing because of |
https://treeherder.mozilla.org/#/jobs?repo=try&revision=211118fed274371548638f3d362e94774d71a19c is it run through Mozilla try; lots of
|
I think I fixed PS: Try https://treeherder.mozilla.org/#/jobs?repo=try&revision=ec6c5777bec884afd30cabfceced9ceaa000c8c6 The failing tests are from the removal of |
I believe that's just a warning and won't block the CL from landing. Nonetheless, I'll look into if I can exclude these third-party files from the check. |
https://chromium-review.googlesource.com/c/chromium/src/+/1636390 updated our lint, thanks @Hexcles! @saschanaz is |
@foolip webidl2.js throws with the format |
external/wpt/IndexedDB/idlharness.any.serviceworker.html from #16864 (comment) explicitly says |
Dismissing my review, as there are some deeper things to look into.
Probably needs a rebase now. |
@foolip Could you run the tests again? Now it should at least show the problematic line number. |
@saschanaz I won't be at my workstation until Monday. Perhaps @lukebjerring can give it a try, otherwise I can next week. |
I've gone through all the regressions on that try run (all fixed by IDL syntax fixes that have landed here but not been synced into m-c, @jgraham), and I'm happy to land this now. I'll give this a day in case anyone disagrees. |
@saschanaz can you explain how you generated the webidl2.js file? |
@Ms2ger |
Filed w3c/webidl2.js#348 to make it straightforward. |
It was added in web-platform-tests#6685 and removed accidentally in web-platform-tests#16864.
Closes #16804, closes #16799