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

path: replace ImmutablePath interface with struct #481

Merged
merged 1 commit into from
Oct 9, 2023

Conversation

Jorropo
Copy link
Contributor

@Jorropo Jorropo commented Oct 6, 2023

Let's not repeat ipfs/go-block-format#45 interface for struct with one implementation and no value added.

Let's not repeat ipfs/go-block-format#45 interface for struct with one implementation and no value added.
@Jorropo Jorropo requested review from lidel, hacdias and a team as code owners October 6, 2023 19:46
@Jorropo
Copy link
Contributor Author

Jorropo commented Oct 6, 2023

Skip changelog since this is not a breaking change yet (we havn't released path yet).

@codecov
Copy link

codecov bot commented Oct 6, 2023

Codecov Report

Merging #481 (7e95df2) into main (85c180e) will increase coverage by 0.03%.
The diff coverage is 42.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #481      +/-   ##
==========================================
+ Coverage   65.73%   65.76%   +0.03%     
==========================================
  Files         207      207              
  Lines       25131    25131              
==========================================
+ Hits        16520    16528       +8     
+ Misses       7149     7143       -6     
+ Partials     1462     1460       -2     
Files Coverage Δ
path/path.go 96.80% <90.00%> (ø)
gateway/handler.go 77.89% <50.00%> (ø)
gateway/blocks_backend.go 43.00% <27.27%> (ø)
gateway/handler_unixfs__redirects.go 48.54% <0.00%> (ø)

... and 11 files with indirect coverage changes

Copy link
Member

@hacdias hacdias left a comment

Choose a reason for hiding this comment

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

I think this change looks sensible. I'm almost sure this was part of the initial PR for the path but I changed it somewhere. Just want to note that Path should stay an interface, and namespaces should be a string. This allows for more experimentation from other people.

But ImmutablePath does indeed make sense to be a struct.

@hacdias hacdias merged commit 45c797e into main Oct 9, 2023
14 of 15 checks passed
@hacdias hacdias deleted the static-immutable-path branch October 9, 2023 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants