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: export routing/http/client.Client #478

Closed
wants to merge 1 commit into from
Closed

Conversation

hacdias
Copy link
Member

@hacdias hacdias commented Oct 4, 2023

Exports Routing V1 client so people can pass it around in their implementations. At the moment it can't be added to structs, or, for example, passed to a function, since it is unexported.

@hacdias hacdias requested a review from a team as a code owner October 4, 2023 11:15
@hacdias hacdias self-assigned this Oct 4, 2023
@codecov
Copy link

codecov bot commented Oct 4, 2023

Codecov Report

Merging #478 (7d7e902) into main (5ae64f3) will increase coverage by 0.09%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #478      +/-   ##
==========================================
+ Coverage   65.69%   65.78%   +0.09%     
==========================================
  Files         208      208              
  Lines       25099    25099              
==========================================
+ Hits        16488    16512      +24     
+ Misses       7152     7136      -16     
+ Partials     1459     1451       -8     
Files Coverage Δ
routing/http/client/client.go 74.59% <100.00%> (ø)

... and 14 files with indirect coverage changes

@@ -28,7 +28,7 @@ import (
)

var (
_ contentrouter.Client = &client{}
_ contentrouter.Client = &Client{}
Copy link
Member

@lidel lidel Oct 4, 2023

Choose a reason for hiding this comment

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

@hacdias Given https://specs.ipfs.tech/routing/http-routing-v1/ maybe call this DelegatedRoutingClient ?

I imagine someone integrating IPFS or experimenting in a single .go file with some PoC may have multiple "clients" (DNS, Kubo RPC, Pinning service, and now this), better to have meaningful names to reduce cognitive load?

Copy link
Member Author

Choose a reason for hiding this comment

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

@lidel hmm, that feels a bit long. I think the main issue here is also the package name. It';s just called client. If it were called something else, it would be much more explicit, like delegatedrouting.Client.

@hacdias
Copy link
Member Author

hacdias commented Oct 4, 2023

Let's just include this in #479.

@hacdias hacdias closed this Oct 4, 2023
@hacdias hacdias deleted the export-routing-client branch October 4, 2023 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants