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

Hide some implementation details from docs #229

Merged
merged 1 commit into from
Nov 1, 2021

Conversation

sivadeilra
Copy link
Contributor

This adds #[doc(hidden)] to several generated items: the vtable
statics for classes, the IID for interfaces, and a type alias.

This makes cargo doc a lot cleaner.

This adds `#[doc(hidden)]` to several generated items: the vtable
statics for classes, the IID for interfaces, and a type alias.

This makes `cargo doc` a lot cleaner.
Copy link
Contributor

@rylev rylev left a comment

Choose a reason for hiding this comment

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

I agree with 2 of the 3 items being hidden, but I think the IID should be left. Thoughts?

@@ -46,6 +46,7 @@ impl IID {
let data4_8 = hex_lit(data4_8);
quote!(
#[allow(missing_docs)]
#[doc(hidden)]
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems less like an implementation detail than the other items. I can certainly imagine someone wanting to directly use an interfaces IID. Are we sure we want this to be hidden?

@MarijnS95
Copy link
Contributor

I agree with 2 of the 3 items being hidden, but I think the IID should be left. Thoughts?

I don't use the docs all that much, but I agree that seems the only thing I'd like to keep.

Afaik com-rs generates function-call helpers for everything so that the user should never touch the vtable manually. On the other hand, certain APIs require an iid+ppv pair to return the desired COM object, and I don't think com-rs hides that in the same way that windows-rs does it currently (by using a type argument and reading T::IID from it)?

@sivadeilra
Copy link
Contributor Author

I think using IFoo::IID or <IFoo as Interface>::IID is better than using IID_IFOO.

@rylev
Copy link
Contributor

rylev commented Nov 1, 2021

I think using IFoo::IID or <IFoo as Interface>::IID is better than using IID_IFOO.

You're right. Ok Let's merge this then.

@rylev rylev merged commit eed7d9c into microsoft:master Nov 1, 2021
@MarijnS95
Copy link
Contributor

I think using IFoo::IID or <IFoo as Interface>::IID is better than using IID_IFOO.

Oh I hadn't read that this is the "static global" declaration, not the associated member on IFoo when implementing the Interface trait. Then this is actually a great nudge to just use that instead 👌

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

Successfully merging this pull request may close these issues.

None yet

3 participants