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

Add "Die On: warning" to spec metadata #468

Closed
inexorabletash opened this issue Oct 6, 2023 · 4 comments · Fixed by #469
Closed

Add "Die On: warning" to spec metadata #468

inexorabletash opened this issue Oct 6, 2023 · 4 comments · Fixed by #469

Comments

@inexorabletash
Copy link
Member

Although not yet documented, Bikeshed now supports setting the default --die-on in the spec metadata. We should make warnings fatal so contributors don't need to remember to specify --die-on=warning manually.

This should just require adding this to the <pre class='metadata'> block at the top of the spec:

Die On: warning

Also, I'm explicitly suggesting "warning" rather than "link-error" because the spec is currently clean for warnings and most Bikeshed warnings really do flag that something is wrong, so we don't want to regress.

@anssiko
Copy link
Member

anssiko commented Oct 10, 2023

@inexorabletash, great idea!

@dontcallmedom, wouldn't a simple BUILD_FAIL_ON: warning in https://github.com/webmachinelearning/webnn/blob/main/.github/workflows/auto-publish.yml#L20C32-L20C32 address this?

(I note the spec-prod example workflow https://github.com/w3c/spec-prod/blob/main/docs/examples.md does not define any exit behaviour on build errors. Should this default be liberal or not? Just wondering.)

@dontcallmedom
Copy link
Contributor

@anssiko
Copy link
Member

anssiko commented Oct 10, 2023

@inexorabletash, maybe we should also fail the CI build similarly as in #469? The in-spec metadata directive will catch local build experiments.

@inexorabletash
Copy link
Member Author

Yes, failing CI is also a good idea.

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 a pull request may close this issue.

3 participants