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

Turn Web IDL into a Living Standard #1018

Merged
merged 2 commits into from Oct 5, 2021
Merged

Turn Web IDL into a Living Standard #1018

merged 2 commits into from Oct 5, 2021

Conversation

annevk
Copy link
Member

@annevk annevk commented Aug 31, 2021

Helps with #1016.


πŸ’₯ Error: 400 Bad Request πŸ’₯

PR Preview failed to build. (Last tried on Sep 3, 2021, 2:15 PM UTC).

More

PR Preview relies on a number of web services to run. There seems to be an issue with the following one:

🚨 CSS Spec Preprocessor - CSS Spec Preprocessor is the web service used to build Bikeshed specs.

πŸ”— Related URL

Error running preprocessor, returned code: 2.
FATAL ERROR: Found unmatched text macro [COMMIT-SHA]. Correct the macro, or escape it with a leading backslash.
 ✘  Did not generate, due to fatal errors

If you don't have enough information above to solve the error by yourself (or to understand to which web service the error is related to, if any), please file an issue.

@annevk annevk mentioned this pull request Aug 31, 2021
19 tasks
@annevk
Copy link
Member Author

annevk commented Aug 31, 2021

A thing I did not realize is that Web IDL has some Node.js-based post processing as well. It's not clear to me if spec-factory's post_build_step is sufficient for that. I suppose it might be...

@annevk
Copy link
Member Author

annevk commented Sep 2, 2021

I essentially added

deploy: index.bs
	curl --remote-name --fail https://resources.whatwg.org/build/deploy.sh
	POST_BUILD_STEP='npm run pp-webidl -- --input index.html' \
	bash ./deploy.sh

to the Makefile to see what kind of errors I would run into. There were a lot. 😟

I ended up suppressing RFC2119 warnings. Given that "optional" is a term that's frequently used that ended up generating too much noise (I suspect "required" as well, but I didn't get that far).

annevk added a commit to whatwg/spec-factory that referenced this pull request Sep 2, 2021
index.bs Outdated Show resolved Hide resolved
@andreubotella
Copy link
Member

andreubotella commented Sep 2, 2021

The default WHATWG boilerplate includes an entry in the spec metadata section that links to the WPT tests, and I noticed that for Web IDL, the WPT folder it links to is webidl, in lowercase, rather than the correct WebIDL. This results in a 404 when you visit the link. The Bikeshed boilerplate should probably include a WPTDIRNAME macro that defaults to the shortname, so it can be overriden if needed.

@annevk annevk marked this pull request as ready for review September 3, 2021 14:46
@annevk
Copy link
Member Author

annevk commented Sep 3, 2021

Really appreciate your help with this @andreubotella!

@domenic want to take a look? @TimothyGu you might also be interested?

@annevk annevk added the do not merge yet Pull request must not be merged per rationale in comment label Sep 3, 2021
.github/workflows/build.yml Show resolved Hide resolved

deploy: index.bs
curl --remote-name --fail https://resources.whatwg.org/build/deploy.sh
POST_BUILD_STEP='node ./check-grammar.js "$$DIR/index.html" && npm run pp-webidl -- --input "$$DIR/index.html"' \
Copy link
Member

Choose a reason for hiding this comment

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

Where does $$DIR come from?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's passed in as an argument. Encoding does the same thing.

@@ -532,7 +476,7 @@ in [[#es-extended-attributes]].
IncludesStatement
</pre>

<div class="example">
<div class="example" id="example-25f60a7c">
Copy link
Member

Choose a reason for hiding this comment

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

Can we give these examples real IDs? I care about this a good bit so if you can't be bothered I guess I'll volunteer :)

Copy link
Member Author

Choose a reason for hiding this comment

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

These are the current IDs, for better or worse. I guess I don't feel too strongly about keeping these IDs working, but folks might be linking to them. (I haven't checked, but either this is disabled or Bikeshed must generate a lot of warnings for the document in its current state.)

Copy link
Member

Choose a reason for hiding this comment

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

Oh, that turns this from a "we should fix before merging" to a "we should maybe fix eventually".

annevk added a commit to whatwg/spec-factory that referenced this pull request Sep 8, 2021
This also makes factory.py more useful for a single standard.

See whatwg/webidl#1018 for context.
@annevk annevk removed the do not merge yet Pull request must not be merged per rationale in comment label Oct 5, 2021
@foolip foolip merged commit 37c0e6f into main Oct 5, 2021
@foolip foolip deleted the annevk/ls branch October 5, 2021 08:17
@foolip
Copy link
Member

foolip commented Oct 5, 2021

Alright, the first deploy worked!
https://webidl.spec.whatwg.org/
https://webidl.spec.whatwg.org/commit-snapshots/37c0e6fe29c50d9c19842135f3f2b265ddcbe932/

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

Successfully merging this pull request may close these issues.

None yet

4 participants