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

static-metric/src/builder: Allow non-public label enums #355

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

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