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

Move WebAssembly reference to the WASM section #20603

Merged
merged 3 commits into from
Sep 19, 2022

Conversation

Josh-Cena
Copy link
Member

Description

See mdn/mdn-community#151

Motivation

Additional details

Related issues and pull requests

@Josh-Cena Josh-Cena requested review from a team as code owners September 12, 2022 19:28
@Josh-Cena Josh-Cena requested review from sideshowbarker and removed request for a team September 12, 2022 19:28
@github-actions github-actions bot added Content:JS JavaScript docs Content:Other Any docs not covered by another "Content:" label Content:wasm WebAssembly docs labels Sep 12, 2022
@github-actions

This comment was marked as resolved.

/en-US/docs/WebAssembly/API/Table/grow /en-US/docs/Web/JavaScript/Reference/Global_Objects/WebAssembly/Table/grow
/en-US/docs/WebAssembly/API/Table/length /en-US/docs/Web/JavaScript/Reference/Global_Objects/WebAssembly/Table/length
/en-US/docs/WebAssembly/API/Table/set /en-US/docs/Web/JavaScript/Reference/Global_Objects/WebAssembly/Table/set
/en-US/docs/WebAssembly/API/Instance /en-US/docs/WebAssembly/JavaScript_interface/Instance
Copy link
Member Author

Choose a reason for hiding this comment

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

Interesting—I didn't know once upon a time these docs were indeed under WebAssembly. I wonder what motivated the change.

Copy link
Collaborator

@hamishwillee hamishwillee Sep 13, 2022

Choose a reason for hiding this comment

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

@Josh-Cena I suspect because these are JavaScript interfaces - WASM might have different APIs in other runtimes. Technically these are probably WebAPIs.

They probably belong in Web APIs, but I think you've done the right thing - WASM is where they are likely to be discovered, and we have a pretty hard rule that the JavaScript section is for language features. I added a note in https://github.com/orgs/mdn/discussions/151#discussioncomment-3630607 to say this.

Copy link
Collaborator

@hamishwillee hamishwillee left a comment

Choose a reason for hiding this comment

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

Thanks for jumping on this.

All the pages have the sidebar macro {{JSRef}} but these will not longer be part of the JSRef tree. So we need to sort these out in the WASM context.

I have not looked at whether there are any jsxrefs mentioning these entities in the source, but we should try fix them if they are.

@Josh-Cena
Copy link
Member Author

Erf, there are a lot of jsxrefs to WebAssembly. I'll try fixing them.

@Josh-Cena
Copy link
Member Author

I've done some crappy regex search & replace and I think I've got all of them.

@hamishwillee
Copy link
Collaborator

Looks good to me. Can you update the groupdata to include Exception and Tag in the sidebar (I missed those when I created them)?

@Josh-Cena
Copy link
Member Author

Josh-Cena commented Sep 13, 2022

Is the WASM sidebar using the GroupData? Looks like it's hardcoded in the macro. Looking at https://github.com/mdn/yari/blob/main/kumascript/macros/WebAssemblySidebar.ejs, we need to stop that one from using jsxref as well.

@hamishwillee
Copy link
Collaborator

Is the WASM sidebar using the GroupData?

Looks like it isn't. Which is a pity in some ways.

Otherwise I think this is good. Would be helpful to have sanity check from @MendyBerger or @sideshowbarker too. @sideshowbarker can you review the yari change?

@hamishwillee
Copy link
Collaborator

@MendyBerger or @sideshowbarker - no comment? I assume you're OK with this and will merge it on Monday if there is no feedback

@Josh-Cena If you can fix the conflicts by Monday I will merge this because I think it is better.

@MendyBerger
Copy link
Contributor

@hamishwillee think this is just moving content around, so I don't really have much to say here.

@hamishwillee
Copy link
Collaborator

Thanks @MendyBerger. That's OK. I'm sure if you thought that this made no sense in terms of a location you'd speak up (i.e. you thought no one would find it).

@hamishwillee
Copy link
Collaborator

Merging. Thanks @Josh-Cena . There might be late feedback from others, but IMO this is clearly better than the current situation.,

@hamishwillee hamishwillee merged commit d606f8d into mdn:main Sep 19, 2022
@Josh-Cena Josh-Cena deleted the mv-webassembly branch September 19, 2022 12:12
himanshugarg pushed a commit to himanshugarg/content that referenced this pull request Sep 27, 2022
* Move WebAssembly reference to the WASM section

* Use WASM sidebar

* Remove jsxref
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:JS JavaScript docs Content:Other Any docs not covered by another "Content:" label Content:wasm WebAssembly docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants