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 http path retrievals #270

Merged
merged 1 commit into from
May 30, 2023
Merged

Fix http path retrievals #270

merged 1 commit into from
May 30, 2023

Conversation

hannahhoward
Copy link
Collaborator

@hannahhoward hannahhoward commented May 29, 2023

Goals

Well folks, I guess this is why we integration test. Looks like we had a very missing trailing slash in the HTTP retrievals which will cause everything except for root cid retrievals to fail.

Implementation

  • Fix it
  • Add integrations tests we never quite got to that sadly, probably would have caught this one

@hannahhoward hannahhoward requested review from rvagg and willscott May 29, 2023 22:47
@codecov-commenter
Copy link

codecov-commenter commented May 29, 2023

Codecov Report

Merging #270 (b862c5d) into main (d87d939) will increase coverage by 0.43%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #270      +/-   ##
==========================================
+ Coverage   70.85%   71.28%   +0.43%     
==========================================
  Files          71       71              
  Lines        6179     6179              
==========================================
+ Hits         4378     4405      +27     
+ Misses       1551     1531      -20     
+ Partials      250      243       -7     
Impacted Files Coverage Δ
pkg/retriever/httpretriever.go 84.95% <100.00%> (ø)

... and 11 files with indirect coverage changes

@hannahhoward hannahhoward merged commit 420763b into main May 30, 2023
@hannahhoward hannahhoward deleted the fix/http-urls branch May 30, 2023 00:47
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