-
Notifications
You must be signed in to change notification settings - Fork 19
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
feat(validate/webidl): add Web IDL validator #104
Conversation
This adds an IDL validation step, enabled by default. The IDL gets extracted from the spec through Reffy and validated with WebIDL2.js. The step gets skipped if no IDL can be extracted from the spec.
@tidoust This looks great! 😍 Thanks for working on this! I'll do testing and review in day tomorrow. I've quickly pushed two small changes to main branch: |
Though, should we disable this check on ReSpec specs as ReSpec already does validation using webidl2? |
Unless I'm missing something, that update does not work on Windows. The call to |
When ReSpec detects invalid IDL, what happens in practice? If the build step fails, then we don't really need to add logic to the IDL validation step: it won't run since build fails. If the build still goes through with some reported errors, then I would actually prefer to keep the IDL validation in this step because it will actually block publication, which is what I think we want here: catch IDL errors early (typically in pull requests) so that they cannot get published in the first place. |
By default, ReSpec build fails on IDL errors and action exits with error, so there's nothing to be done. But if
Lets also skip it for now then 😄 |
Co-authored-by: Sid Vishnoi <8426945+sidvishnoi@users.noreply.github.com>
Co-authored-by: Sid Vishnoi <8426945+sidvishnoi@users.noreply.github.com>
Co-authored-by: Sid Vishnoi <8426945+sidvishnoi@users.noreply.github.com>
Co-authored-by: Sid Vishnoi <8426945+sidvishnoi@users.noreply.github.com>
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.
SHA: e228dcc Reason: push, by @sidvishnoi Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Released in v2 ≡ v2.6.0 🎉 |
This adds an IDL validation step, enabled by default. The IDL gets extracted from the spec through Reffy and validated with WebIDL2.js.
The step gets skipped if no IDL can be extracted from the spec.
Enabling the step by default should not have any noticeable impact on "well-known" specs, typically those that are in browser-specs. In particular, the IDL patches that we currently need to maintain in Webref are for specs that do not use spec-prod, except for the DeviceOrientation Event specification, whose IDL fix is being tracked in w3c/deviceorientation#88.
@sidvishnoi, FYI, I note that, in order to test locally, and on top of the changes you suggested in #93 (comment), I also needed to:
PUPPETEER_EXECUTABLE_PATH
env variable to an empty string in src/build.ts line 182 through:undefined
.GITHUB_ENV
andGITHUB_PATH
env varibles to an empty string in test/index.test.ts line 11 instead of/dev/null
(perhaps simply because I'm on a Windows machine)This would fix #87.