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

enable all virtual file systems #486

Merged
merged 2 commits into from
Aug 3, 2023
Merged

enable all virtual file systems #486

merged 2 commits into from
Aug 3, 2023

Conversation

maxfreu
Copy link
Contributor

@maxfreu maxfreu commented Aug 3, 2023

Hi! I wanted to use gdal's /vsitar virtual file system and found only checks for curl and mem, so I added the rest. I also modified the if condition a bit to use startswith. I haven't added any test; the best I could do is to provide a tar file and check with that. But I won't be able to check s3, az etc as I don't have any data there.

@rafaqz
Copy link
Owner

rafaqz commented Aug 3, 2023

Great! maybe put all the virtual file system strings in a const above the function to keep things clean?

I'm ok with not testing them, its not really feasable and we test vsicurl already.

@maxfreu
Copy link
Contributor Author

maxfreu commented Aug 3, 2023

Done!

@codecov-commenter
Copy link

codecov-commenter commented Aug 3, 2023

Codecov Report

Merging #486 (d4eed95) into main (b2e06e9) will decrease coverage by 0.22%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #486      +/-   ##
==========================================
- Coverage   81.25%   81.04%   -0.22%     
==========================================
  Files          49       49              
  Lines        4135     4126       -9     
==========================================
- Hits         3360     3344      -16     
- Misses        775      782       +7     
Files Changed Coverage Δ
ext/RastersArchGDALExt/gdal_source.jl 88.84% <100.00%> (ø)

... and 8 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@rafaqz rafaqz merged commit f36ba3f into rafaqz:main Aug 3, 2023
5 checks passed
@rafaqz
Copy link
Owner

rafaqz commented Aug 3, 2023

Thanks!

@maxfreu
Copy link
Contributor Author

maxfreu commented Aug 3, 2023

You're welcome :)

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.

3 participants