Skip to content

Conversation

@Elchi3
Copy link
Collaborator

@Elchi3 Elchi3 commented Oct 15, 2024

Taking over #1634

Specification URLs are tricky here. Not every feature has a proper spec or is integrated in the main wasm spec. Often there is just a markdown file to point to (BCD does this and we then have to add lots of URLs as exceptions for spec urls).

@github-actions github-actions bot added the feature definition Creating or defining new features or groups of features. label Oct 15, 2024
@Elchi3 Elchi3 mentioned this pull request Oct 15, 2024
Copy link
Collaborator

@autonome autonome left a comment

Choose a reason for hiding this comment

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

Another pass on descriptions, but haven't done a looksee into Caniuse. Are there any discrepancies there?

Copy link
Collaborator

@autonome autonome left a comment

Choose a reason for hiding this comment

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

A couple more non-blocking description suggestions.

@Elchi3 Elchi3 requested a review from ddbeck October 29, 2024 14:50
Copy link
Collaborator

@ddbeck ddbeck left a comment

Choose a reason for hiding this comment

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

Partial review here. Main points so far:

  • The "An extension to…" structure of these descriptions is a major departure from the rest of our descriptions. Other descriptions are complete sentences. I've made some suggestions to restructure these, but the unhappy side effect of having to write a complete sentence is that you have to know what you're saying and when it comes to wasm, I do not.

  • For the IDs, I'm fine with wasm- prefixes, but for the names, I'd prefer to preserve the scanability of a list by either dropping the WebAssembly prefix (if it makes sense to do so) or moving it to a disambiguating suffix (e.g., Tail call optimzation (WebAssembly).

  • I had no idea about the webassembly.api namespace in BCD. That is… unusual and surprising.

@Elchi3
Copy link
Collaborator Author

Elchi3 commented Nov 4, 2024

Thanks Daniel! Without the specs/explainers, it is really hard to tell what these things are. I pushed fe5acf7 to update the descriptions and to add spec urls.

I think I still need/want to:

  • tell the linter to not complain about these urls
  • (optional) group all these features in a "webassembly" group

@ddbeck
Copy link
Collaborator

ddbeck commented Nov 4, 2024

@Elchi3 Great, thank you!

You can add exceptions here:

const defaultAllowlist: allowlistItem[] = [
// [
// "https://example.com/spec/",
// "Allowed because…. Remove this exception when https://example.com/org/repo/pull/1234 merges."
// ]

Though if BCD is already doing this, I suppose we could "inherit" BCD's overrides on some automatic basis.

(Now that I'm saying this, checking the validity of a spec URL and fragment might make a nice little standalone package that we could share between BCD and web-features.)

@github-actions github-actions bot added the tools and infrastructure Project internal tooling, such as linters, GitHub Actions, or repo settings label Nov 4, 2024
@Elchi3
Copy link
Collaborator Author

Elchi3 commented Nov 4, 2024

Thanks for the quick response! I added to the exception list and also grouped the features.

(I'm improving spec URL validation even further in BCD [to also guarantee fragment validity with webref] but maybe web-specs itself could offer some such validations for all consumers [or, as you say, at some point we move this into a standalone package])

Copy link
Collaborator

@ddbeck ddbeck left a comment

Choose a reason for hiding this comment

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

OK, I've suggested some more modifications to the descriptions. I think we're close. Take what you like and ignore the rest. Like the WebGL extensions, as long as we're giving folks a clue that this is not relevant to most web developers, then we'll probably be OK.

There is one thing that I feel moderately strong about: I'd like to preserve the existing wasm-simd ID, unless you think it's really worthy of a breaking release (or have other breaking changes you want to make soon).

@Elchi3 Elchi3 requested a review from ddbeck November 5, 2024 11:07
Copy link
Collaborator

@ddbeck ddbeck left a comment

Choose a reason for hiding this comment

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

Let's go!

@ddbeck ddbeck merged commit 1d7f6b0 into web-platform-dx:main Nov 5, 2024
3 checks passed
@Elchi3 Elchi3 deleted the wasm branch November 5, 2024 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature definition Creating or defining new features or groups of features. tools and infrastructure Project internal tooling, such as linters, GitHub Actions, or repo settings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants