search

queryverse / ElectronDisplay.jl

An Electron.jl based figure and table display.
Other
85 stars 17 forks source link

Update FilePaths.jl bounds #80

Closed davidanthoff closed 3 years ago

davidanthoff commented 3 years ago

Fixes https://github.com/queryverse/ElectronDisplay.jl/issues/79. Or at least I hope.

I think FilePaths.jl v0.8.1 should guaranty that we get a version of FilePathsBase.jl that is based on joinpath.

But I can't do anything about the version that has already been released that specifies compat as FilePaths.jl 0.8... That is a real problem, because with that compat entry one can get either FilePathsBase 0.6 or 0.9, and those have breaking differences... So I think the core problem is that FilePaths.jl 0.8.1 should really have been FilePaths.jl 0.9...

ericphanson commented 3 years ago

For what it's worth, I agree that FilesPath should have had a 0.9 release. It seems natural that FilePaths' public API includes the functionality provided by FilesPathBase, and that breaking changes there are also breaking for FilePaths.

This kind of thing has come up in Slack a few times as a subtlety of compat in the face of multiple dispatch (in that your public API can include extensions of Base functions and you can "leak" the API of your dependencies), and I think the solution is community norms about communicating your public API and being aware of what behavior of your dependencies your users are counting on when using your package.

rofinn commented 3 years ago

FWIW, I think the bigger issue is that FilePaths.jl shouldn't re-export FilePathsBase.jl. The general convention is that updating a packages to handle new dependency versions should be patch releases if it doesn't impact the API in any way. That's largely true for FilePaths.jl releases... apart from the fact that we're using @re-export. Since that basically ties API changes of FilePathsBase.jl and FilePaths.jl I don't think that approach should continue. Instead, the base API should be independent of the convenience methods in FilePaths.jl.

In this particular case, I probably should have double checked what changed in FilePathsBase.jl before releasing 0.8.1, but honestly I just forgot about the re-exporting behaviour.