Skip to content
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

Adding WebIDL validation to auto-publish workflow #2518

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

hoch
Copy link
Member

@hoch hoch commented Oct 11, 2022

This PR adds a new step in the auto-publish workflow to use the WebIDL validation. It also updates some old version of GitHub actions to the latest ones.

Context: #2514 (comment)

@sidvishnoi
Copy link

You can use spec-prod to publish to GitHub pages too btw. You won't need to setup bikeshed, and other GitHub actions.

@hoch
Copy link
Member Author

hoch commented Oct 11, 2022

Yes, that what I tried first a year ago. However, our spec has some "unfixable" Bikeshed issues, so we had to use our own build flow to avoid spec-prod. Now I remember why we took this approach.

Thanks for your pointer! I'll leave this open so I don't forget why we're using a custom spec building action.

@@ -23,6 +23,8 @@ jobs:
- name: Building index.html from index.bs
run: bash ./compile.sh
shell: bash
- name: Running WebIDL validation
uses: w3c/spec-prod@v2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you just want to use spec-prod to validate the IDL, perhaps you could use the BUILD_FAIL_ON spec-prod action setting to have Bikeshed die on nothing?

That won't give you an occasion to validate that the list of errors returned by Bikeshed is the list you expect, as done in the compile script, so probably not a good idea for actual publication to GitHub Pages.

Suggested change
uses: w3c/spec-prod@v2
uses: w3c/spec-prod@v2
with:
BUILD_FAIL_ON: nothing

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the tip! That might be a way to resolve this. I'll try to incorporate this idea to our workflow.

Just to be clear, spec-prod@v2 already includes Bikeshed and we don't need a custom step to run Bikeshed (L23~L25). Right?

@padenot I believe we're doing the text comparison (actual vs error) for some historical reasons. Do you have context on why we can't fix those Bikeshed errors? Wanted to ask before I jump in and investigate.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to be clear, spec-prod@v2 already includes Bikeshed and we don't need a custom step to run Bikeshed (L23~L25). Right?

Correct. If the Bikeshed errors are fixed or ignored (with BUILD_FAIL_ON), spec-prod can fully replace the current workflow. You'll only need checkout step and spec-prod step then (spec-prod handles Python and Bikeshed and all).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hoch, last I checked, with Raymond, some time ago, there were shortcomings/bugs in bikeshed, preventing us to fix those, but of course they might well be fixed now.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you both for clarification!

@jackromo888
Copy link

adding-idl-validation-action

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants