Skip to content

[wasm] add missing API xml doc #76196

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Oct 3, 2022
Merged

[wasm] add missing API xml doc #76196

merged 4 commits into from
Oct 3, 2022

Conversation

@pavelsavara
Copy link
Member

@pavelsavara pavelsavara commented Sep 26, 2022

Fixes #74734

@pavelsavara pavelsavara added this to the 8.0.0 milestone Sep 26, 2022
@pavelsavara pavelsavara self-assigned this Sep 26, 2022
@ghost
Copy link

ghost commented Sep 26, 2022

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@ghost
Copy link

ghost commented Sep 26, 2022

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #74734

Author: pavelsavara
Assignees: pavelsavara
Labels:

arch-wasm, area-System.Runtime.InteropServices.JavaScript
Milestone: 8.0.0

@pavelsavara
Copy link
Member Author

pavelsavara commented Sep 26, 2022

@carlossanlop how do I run the completeness check on this ?

@carlossanlop
Copy link
Contributor

carlossanlop commented Sep 26, 2022

@carlossanlop how do I run the completeness check on this ?

The undoc APIs report is generated in dotnet-api-docs, not on runtime. So the only way to validate that this PR is complete, is by making sure the list of undoc APIs I provided in the issue is covered in this PR.

Please help port the undoc APIs to dotnet-api-docs as well. That repo is the source of the intellisense package we ship to customers. We generate it there after the text goes through language review.

@carlossanlop
Copy link
Contributor

carlossanlop commented Sep 26, 2022 •
edited
Loading

@pavelsavara to port your triple slash comments to dotnet-api-docs, use the https://github.com/dotnet/api-docs-sync/ tool.

  1. Install the tool using the .\install-as-tool.ps1 script. The tool is PortToDocs.exe.
  2. For your specific case, you need to pre-build your runtime repo for wasm, so the intellisense xml file is generated for that architecture.
  3. Clone the dotnet-api-docs repo locally.
  4. Example of the command you would execute:
PortToDocs.exe -Docs D:\dotnet-api-docs\xml -IntelliSense D:\runtime7\artifacts\bin\System.Runtime.InteropServices.JavaSc
ript\Release\net7.0-browser -IncludedAssemblies "System" -IncludedNamespaces "System" -Save true

Assumptions of the example:

  • Your repos live in the D:\ drive.
  • You built runtime in Release for browser.
  • Your undoc APIs fullname DocIDs start with System, both in the assembly and the namespace.

carlossanlop
carlossanlop reviewed Sep 26, 2022
View reviewed changes
Copy link
Contributor

@carlossanlop carlossanlop left a comment

Choose a reason for hiding this comment

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

Left some suggestions for you to consider.

@pavelsavara pavelsavara mentioned this pull request Sep 27, 2022
Merged
@pavelsavara
Copy link
Member Author

pavelsavara commented Sep 27, 2022

I created dotnet/dotnet-api-docs#8436 and generated the docs into via the tool.
The tools also gives feedback about missing docs!

I didn't know how to create namespace doc in the runtime repo so I wrote it by hands in the dotnet-api-docs PR

@pavelsavara pavelsavara marked this pull request as ready for review September 27, 2022 18:28
@pavelsavara pavelsavara requested a review from lewing as a code owner September 27, 2022 18:28
@pavelsavara pavelsavara marked this pull request as draft September 28, 2022 00:26
@carlossanlop
Copy link
Contributor

carlossanlop commented Sep 29, 2022

I didn't know how to create namespace doc in the runtime repo so I wrote it by hands in the dotnet-api-docs PR

Thanks. Yep, that was the right way to do it. It can't be ported.

carlossanlop
carlossanlop reviewed Sep 29, 2022
View reviewed changes
@pavelsavara pavelsavara marked this pull request as ready for review September 30, 2022 13:35
@pavelsavara
Copy link
Member Author

pavelsavara commented Sep 30, 2022

many thanks @carlossanlop

@pavelsavara pavelsavara merged commit ff8ac3b into dotnet:main Oct 3, 2022
@pavelsavara pavelsavara deleted the wasm_missing_api_docs branch October 13, 2022 10:04
@ghost ghost locked as resolved and limited conversation to collaborators Nov 12, 2022
@ericstj ericstj added the breaking-change Issue or PR that represents a breaking API or functional change over a prerelease. label Mar 30, 2023
@ghost ghost added the needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet label Mar 30, 2023
@ericstj
Copy link
Member

ericstj commented Mar 30, 2023

Marked this as breaking since it removes an implicit public default constructor. cc @lewing

@pavelsavara pavelsavara removed the needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet label Oct 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@maraf maraf maraf approved these changes

@gewarren gewarren Awaiting requested review from gewarren

@lewing lewing Awaiting requested review from lewing lewing is a code owner

@carlossanlop carlossanlop Awaiting requested review from carlossanlop

Assignees

@pavelsavara pavelsavara

Labels

arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript breaking-change Issue or PR that represents a breaking API or functional change over a prerelease. new-api-needs-documentation

Projects

None yet

Milestone

8.0.0

Development

Successfully merging this pull request may close these issues.

Undocumented 7.0 APIs in System.Runtime.InteropServices.Javascript

4 participants

@pavelsavara @carlossanlop @ericstj @maraf