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

refactor(core/routing): split ContentRouting interface #3048

Merged
merged 1 commit into from
Nov 19, 2024

Conversation

hsanjuan
Copy link
Contributor

@hsanjuan hsanjuan commented Nov 19, 2024

The ContentRouting interface does too much. One can think of systems that only Provide, and systems that only FindProviders.

Therefore I am proposing to split it further. While this can be done on user-code, core/routing interfaces are canonical and it is better to use interfaces coming from here.

As someone said: the best interface is that which does as little as possible.

core/routing/routing.go Outdated Show resolved Hide resolved
@willscott
Copy link
Contributor

Chiming in to say I agree with this direction - IPNI is a good example of a system where we've run into issues with there being a desire for the system to support provides as IPFS defines in delegated routing as part of being able to be used for queries.

The ContentRouting interface does too much. One can think of systems that only
Provide, and systems that only FindProviders.

Therefore I am proposing to split it further. While this can be done on
user-code, core/routing interfaces are canonical and it is better to use
interfaces coming from here.

As someone said: the best interface is that which does as little as possible.
@hsanjuan hsanjuan force-pushed the split-content-routing-interface branch from 731e87e to 7ee1eaa Compare November 19, 2024 13:31
@sukunrt sukunrt requested a review from MarcoPolo November 19, 2024 13:55
Copy link
Member

@sukunrt sukunrt left a comment

Choose a reason for hiding this comment

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

I'm fine with the change, but will let Marco opine as he has more understanding of this side of the codebase.

@MarcoPolo MarcoPolo changed the title Split ContentRouting interface refactor(core/routing): split ContentRouting interface Nov 19, 2024
@MarcoPolo MarcoPolo merged commit e4db464 into master Nov 19, 2024
11 checks passed
@hsanjuan hsanjuan deleted the split-content-routing-interface branch November 20, 2024 13:31
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.

4 participants