Skip to content

Conversation

mxinden
Copy link
Contributor

@mxinden mxinden commented Oct 19, 2020

Imagine the following static metric generation:

make_static_metric! {
    label_enum Methods {
        post,
        get,
    }

    struct MyStaticCounterVec: Counter {
        "method" => Methods,
    }
}

This will roughly expand to:

enum Methods {
    post,
    get,
}

use self::prometheus_static_scope_0::MyStaticCounterVec;
mod prometheus_static_scope_0 {
    pub struct MyStaticCounterVec {
        pub post: MyStaticCounterVec2,
        pub get: MyStaticCounterVec2,
    }
}

Rustc would complain that the private type 'Methods' [is] in [the] public interface (error E0446) MyStaticCounterVec.

There is no reason for MyStaticCounterVec to be defined with
visibility pub. This commit instead defines MyStaticCounterVec with
pub(super) and thus MyStaticCounterVec does not expose the private
type Methods.

Signed-off-by: Max Inden [email protected]

Imagine the following static metric generation:

```rust
make_static_metric! {
    label_enum Methods {
        post,
        get,
    }

    struct MyStaticCounterVec: Counter {
        "method" => Methods,
    }
}
```

This will roughly expand to:

```rust
enum Methods {
    post,
    get,
}

use self::prometheus_static_scope_0::MyStaticCounterVec;
mod prometheus_static_scope_0 {
    pub struct MyStaticCounterVec {
        pub post: MyStaticCounterVec2,
        pub get: MyStaticCounterVec2,
    }
}
```

Rustc would complain that the `private type 'Methods' [is] in [the]
public interface (error E0446)` `MyStaticCounterVec`.

There is no reason for `MyStaticCounterVec` to be defined with
visibility `pub`. This commit instead defines `MyStaticCounterVec` with
`pub(super)` and thus `MyStaticCounterVec` does not expose the private
type `Methods`.

Signed-off-by: Max Inden <[email protected]>
@mxinden mxinden added the bugfix label Oct 19, 2020
Copy link
Member

@breezewish breezewish left a comment

Choose a reason for hiding this comment

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

Thanks!

@breezewish
Copy link
Member

Maybe supplying some tests that will pass compile in this PR but will fail in master would be better!

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

Successfully merging this pull request may close these issues.

2 participants