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

feat: re-export prost #256

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

Conversation

yjhmelody
Copy link

@yjhmelody yjhmelody commented Jan 14, 2025

The following code is very common:

    let mut buffer = Vec::new();
    let metrics_set = prometheus_client::encoding::protobuf::encode(&state.registry)?;
    prost::Message::encode(&metrics_set, &mut buffer)?;
    Ok(buffer)

But we need to import prost crate

Signed-off-by: yjhmelody <[email protected]>
@cratelyn
Copy link
Contributor

is there any reason why you cannot also depend on prost? exposing these types in the public interface of this crate introduces a worrisome footgun with respect to semantic versioning: updating this crate's version of prost would be a breaking change.

doing this would mean that this library would need to pin itself to a particular version of prost, and would in turn be prescriptive about what version of prost users of this library to a particular version as well.

there's some discussion about this on the rust forum here: https://users.rust-lang.org/t/semver-behavior-for-dependency-changes/73665/2

If you bump the version of public dependency (i.e. of something which appears in your API), however, that's the different story. In general, this change would be breaking, so you have to bump your own version too.

@mxinden
Copy link
Member

mxinden commented Jan 15, 2025

Unless this is recommended by the Rust project, I would prefer not to re-export crates in our public API, for the sake of simplicity. In this particular case especially, as I would prefer to get rid of prost.

#162 (comment)

@mxinden
Copy link
Member

mxinden commented Jan 15, 2025

updating this crate's version of prost would be a breaking change.

Unless I am missing something, bumping to a breaking version of prost is already a breaking change for this crate.

See e.g. prost in our public API:

https://docs.rs/prometheus-client/latest/prometheus_client/encoding/protobuf/openmetrics_data_model/struct.CounterValue.html#impl-Message-for-CounterValue

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.

3 participants