Initial import #5
Conversation
|
Build failing because github still only has Rust 1.78. Might not be a bad idea to support 1.78 for a while, even though it requires depending on an extra external crate. |
Yep, I'm intending on doing that in a new iteration today. |
- Make the provider group type fixed per-platform rather than an enum (yes, it is still an Option enum) - Make the guid type a common wrapper - Make the platform crates only used on that platform
| use crate::native::ProviderTypes; | ||
|
|
||
| // Public with public fields because the `etw_event!` macro needs to create it at invocation site. | ||
| #[doc(hidden)] |
There was a problem hiding this comment.
Shouldn't need another doc(hidden) since the whole module was marked hidden.
|
|
||
| // This struct needs to be public as it implements the tracing_subscriber::Layer trait. | ||
| #[doc(hidden)] | ||
| pub struct EtwLayer<S, Mode: ProviderTypes> |
There was a problem hiding this comment.
Consider: instead of defining this here, move it to layer.rs, then re-export it from here.
There was a problem hiding this comment.
If I want to export it from here, then it needs to be public in layer.rs too - I can't increase the visibility of it through a use statement. And if it's public in layer.rs, then we're back where we started.
There was a problem hiding this comment.
Sure. But the layer module itself isn't pub. (Though this does mean you would need to stop using * to export items from layer.)
| // This struct needs to be public as it implements the tracing_subscriber::Layer::Filter trait. | ||
| #[doc(hidden)] | ||
| #[cfg(not(feature = "global_filter"))] | ||
| pub struct EtwFilter<S, Mode: ProviderTypes> |
There was a problem hiding this comment.
Consider: instead of defining this here, move it to layer.rs, then re-export it from here.
|
|
||
| #[allow(non_camel_case_types)] | ||
| #[derive(Default, Clone)] | ||
| #[doc(hidden)] |
There was a problem hiding this comment.
Shouldn't need doc(hidden) since the module isn't pub.
| /// Get the current provider ID that will be used for the ETW provider. | ||
| /// This is a convenience function to help with tools that do not implement | ||
| /// the standard provider name to ID algorithm. | ||
| pub fn get_provider_id(&self) -> &GuidWrapper { |
There was a problem hiding this comment.
Consider returning a u128 here so caller doesn't need to know what a GuidWrapper is.
There was a problem hiding this comment.
I forgot that I wanted to come back to the guids.
Docs are still a work in progress. Will address comments from #4 in a new iteration on this PR.