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

Fix re-export of TEXT_FORMAT to not require protobuf #416

Merged
merged 1 commit into from Oct 11, 2021
Merged

Fix re-export of TEXT_FORMAT to not require protobuf #416

merged 1 commit into from Oct 11, 2021

Conversation

teliosdev
Copy link
Contributor

A minor change, but previously the root-level re-export of TEXT_FORMAT would require cfg(feature = "protobuf"), despite the text export format not requiring protobufs. Since this is the only location where TEXT_FORMAT is made public, it should be exported regardless of protobuf feature availability.

@lucab
Copy link
Member

lucab commented Oct 5, 2021

Thanks for the PR, good catch!
Can you please amend your commit with a sign-off line? (See DCO check in CI for details)

A minor change, but previously the root-level re-export of
TEXT_FORMAT would require cfg(feature = "protobuf"), despite
the text export format not requiring protobufs.  Since this is
the only location where TEXT_FORMAT is made public, it should
be exported regardless of protobuf feature availability.

Signed-off-by: Jeremy Rodi <me@telios.dev>
@lucab lucab merged commit 99b6064 into tikv:master Oct 11, 2021
@lucab lucab mentioned this pull request Apr 21, 2022
JanBerktold pushed a commit to JanBerktold/rust-prometheus that referenced this pull request Nov 12, 2022
A minor change, but previously the root-level re-export of
TEXT_FORMAT would require cfg(feature = "protobuf"), despite
the text export format not requiring protobufs.  Since this is
the only location where TEXT_FORMAT is made public, it should
be exported regardless of protobuf feature availability.

Signed-off-by: Jeremy Rodi <me@telios.dev>
Signed-off-by: Jan Berktold <jberktold@roblox.com>
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

2 participants