-
Notifications
You must be signed in to change notification settings - Fork 19
Add uri extension #186
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
Add uri extension #186
Conversation
|
It looks like the tests are failing because the URI version is causing the tests to use an old version of BSON (v0.2.6 vs v0.3.9). I'll need to look at adjusting the dependency versions to fix this. Otherwise, I don't see an issue. Would you mind reminding me why it makes sense to include the extension here of FilePaths.jl? I know you opened an issue but didn't see any PRs. @davidanthoff I think you suggested having the extension here? |
|
Oh, I think I figured out what is happening. We're using JLSO.jl just as a test case, but JLSO.jl actively depends on FilePathsBase.jl. Bumping the FilePathsBase.jl release breaks the existing compatibility. |
|
Hm, I have to admit I don't even know whether the "recommended" approach would be to have it here or in URIs.jl? Between here and FilePaths.jl, I also don't really know. I think if we were to start today, we wouldn't have FilePathsBase.jl at all, right? Maybe the right strategy would actually be to move everything over into FilePaths.jl and give up on FilePathsBase.jl? I think for the short-term fix it probably doesn't matter? |
|
Agreed. There would have been no need for FilePathsBase.jl if we had the 1.9 package extensions in the 0.5 days. In any case, I've made a PR on FilePaths.jl, hoping to address the immediate blocker. CI jobs were disabled, but test pass locally and I've re-enabled the jobs for prior to tagging. (FilePaths) pkg> test
Testing FilePaths
Status `/private/var/folders/sy/srj5df5j7jvf0_hb225496sh0000gn/T/jl_qLOJSi/Project.toml`
[8fc22ac5] FilePaths v0.9.0 `~/repos/FilePaths.jl`
[48062228] FilePathsBase v0.9.21
[c27321d9] Glob v1.3.1
[1914dd2f] MacroTools v0.5.13
[189a3867] Reexport v1.2.2
[30578b45] URIParser v0.4.1
[5c2747f8] URIs v1.5.1
[8dfed614] Test v1.11.0
Status `/private/var/folders/sy/srj5df5j7jvf0_hb225496sh0000gn/T/jl_qLOJSi/Manifest.toml`
[34da2185] Compat v4.14.0
[8fc22ac5] FilePaths v0.9.0 `~/repos/FilePaths.jl`
[48062228] FilePathsBase v0.9.21
[c27321d9] Glob v1.3.1
[1914dd2f] MacroTools v0.5.13
[189a3867] Reexport v1.2.2
[30578b45] URIParser v0.4.1
[5c2747f8] URIs v1.5.1
[2a0f44e3] Base64 v1.11.0
[ade2ca70] Dates v1.11.0
[b77e0a4c] InteractiveUtils v1.11.0
[56ddb016] Logging v1.11.0
[d6f4376e] Markdown v1.11.0
[a63ad114] Mmap v1.11.0
[de0858da] Printf v1.11.0
[9a3f8284] Random v1.11.0
[ea8e919c] SHA v0.7.0
[9e88b42a] Serialization v1.11.0
[fa267f1f] TOML v1.0.3
[8dfed614] Test v1.11.0
[cf7118a7] UUIDs v1.11.0
[4ec0a83e] Unicode v1.11.0
Precompiling project for configuration --code-coverage=none --color=yes --check-bounds=yes --warn-overwrite=yes --depwarn=yes --inline=yes --startup-file=no --track-allocation=none...
3 dependencies successfully precompiled in 1 seconds. 20 already precompiled.
Testing Running tests...
┌ Warning: `URIParser` is deprecated, use `URIs` instead.
│ caller = ip:0x0
└ @ Core :-1
Test Summary: | Pass Total Time
FilePaths | 36 36 3.5s
Testing FilePaths tests passedI'll make a separate issue to address the weird testing compat issue between FilePathsBase.jl and JLSO.jl. Honestly, might just drop the dependency since I don't think anyone outside of Invenia used that package anyway. Longer term, I'm fine with moving the FilePathsBase.jl code over to FilePaths.jl. Also, might fit nicely with switching substring storage and bumping to 1.0 for easier compat handling. |
|
@frankier I've opened, merged and tagged a requires -> extensions PR on FilePaths.jl for now. Assuming the tag goes through, will that be enough to unblock you for now? I'll leave addressing the larger ecosystem / dependency issues to another night. |
|
I think this should do the trick. We have to check if Makie loads faster but it should do. Thanks so much! |
Continuing from davidanthoff/Electron.jl#140
This PR adds a package extension for URIs.jl. The implementation is taken from FilePaths.jl.
I think the version needs to be bumped so FilePaths.jl does not pull this in.