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

Export the definition of Change in tower::discover rather than rewriting it. #2064

Closed
wants to merge 1 commit into from

Conversation

accestfromim
Copy link

Export the definition of Change in tower::discover rather than rewriting it.

Motivation

the issue #2049 reported a bug but I couldn't reproduce it. However, while trying to reproduce it, I noticed that the definition of enum Change is duplicated, so I was thinking if should export it instead of rewriting it.

Solution

before:
n tonic/src/transport/channel/service/discover.rs line 14-20

#[derive(Debug, Clone)]
pub enum Change<K, V> {
    /// A new service identified by key `K` was identified.
    Insert(K, V),
    /// The service identified by key `K` disappeared.
    Remove(K),
}

and it is just the same as in tower::discover::Change

after:

pub use tower::discover::Change;

Export the definition of Change in tower::discover
rather than rewriting it.
@tottoto
Copy link
Collaborator

tottoto commented Nov 23, 2024

It is an intentional design not to expose tower crate's type as external API. You should track the history and try to find how they were intended at first, when you have any questions about the implementations.

@tottoto tottoto closed this Nov 23, 2024
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.

2 participants