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 documentation for ICorProfilerInfo13 methods and enumeration. #30131

Merged
merged 28 commits into from
Jul 29, 2022

Conversation

chrisnas
Copy link
Contributor

@chrisnas chrisnas commented Jul 7, 2022

Summary

Provide the documentation for the new ICorProfilerInfo13 interface implemented by dotnet/runtime#71257

Describe your changes here.
Add the interface, methods and enumeration documentation

Fixes #Issue_Number (if available)

Copy link
Contributor

@gewarren gewarren left a comment

Choose a reason for hiding this comment

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

I left a few grammar improvements. Also, should the enumeration be added to profiling-enumerations.md?

chrisnas and others added 18 commits July 8, 2022 09:36
…bjectidfromhandle-method.md

Co-authored-by: Genevieve Warren <[email protected]>
…bjectidfromhandle-method.md

Co-authored-by: Genevieve Warren <[email protected]>
…bjectidfromhandle-method.md

Co-authored-by: Genevieve Warren <[email protected]>
@chrisnas
Copy link
Contributor Author

chrisnas commented Jul 8, 2022

I left a few grammar improvements.

I would have probably done less errors in french :^)

Also, should the enumeration be added to profiling-enumerations.md?

Good point. I'm udpating it.

I also picked a format for the link to .NET 7 content based on .NET 5 one
[!INCLUDE[net_core](../../../../includes/net-core-70-md.md)]
Let me know if this needs to be changed

@gewarren
Copy link
Contributor

gewarren commented Jul 8, 2022

I also picked a format for the link to .NET 7 content based on .NET 5 one [!INCLUDE[net_core](../../../../includes/net-core-70-md.md)] Let me know if this needs to be changed

The link is correct, but that include file doesn't exist, so you'll need to add it.

@gewarren gewarren closed this Jul 8, 2022
@chrisnas
Copy link
Contributor Author

chrisnas commented Jul 9, 2022

@gewarren: why did you close this PR?
Is there anything missing (except for the net-core-70-md.md file)?

@gewarren
Copy link
Contributor

gewarren commented Jul 9, 2022

Sorry, not sure what happened.

@gewarren gewarren reopened this Jul 9, 2022
@chrisnas
Copy link
Contributor Author

chrisnas commented Jul 9, 2022

Sorry, not sure what happened.

I did the same thing by mistake on my own PR so don't be sorry ;^)
I'm leaving for 1 week vacation so expect the requested changes to be done only from july 19th

@chrisnas
Copy link
Contributor Author

I also picked a format for the link to .NET 7 content based on .NET 5 one [!INCLUDE[net_core](../../../../includes/net-core-70-md.md)] Let me know if this needs to be changed

The link is correct, but that include file doesn't exist, so you'll need to add it.

I just added the missing file

@gewarren gewarren requested a review from davmason July 19, 2022 20:53
Copy link
Member

@davmason davmason left a comment

Choose a reason for hiding this comment

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

LGTM

@gewarren gewarren merged commit 0133630 into dotnet:main Jul 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants