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

Update to the new release of FilePathsBase #40

Merged
merged 2 commits into from
Jul 30, 2019
Merged

Conversation

rofinn
Copy link
Owner

@rofinn rofinn commented Jul 26, 2019

This depends on the new FilePathsBase version being released.

Copy link
Contributor

@oxinabox oxinabox left a comment

Choose a reason for hiding this comment

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

Are we accessing fields on AbstractFilePaths?

@rofinn
Copy link
Owner Author

rofinn commented Jul 27, 2019

Sort of. All types should have at least have a field called segments and everything else like drive and root will have abstract defaults.

@oxinabox
Copy link
Contributor

Can you fill me in on why the use of getproperty and having required fields on all subtypes of an abstract type is better?
I liked the way it was before, I feel like I can trust function calls.

@rofinn
Copy link
Owner Author

rofinn commented Jul 28, 2019

It’s mostly a namespacing issue for generic names like root, drive and parts. Also, parts and segments have different behaviour now. The reason this changed is that getproperty can be used as a way of handling depreciations now.

@oxinabox
Copy link
Contributor

hmm, the namespacing use-case is interesting. Still not sure how I feel about this though.

@rofinn
Copy link
Owner Author

rofinn commented Jul 28, 2019

That's part of the reason we don't have a 1.0 release yet. I figured we should try the getproperty approach for a bit to see how that works. If it's too awkward we can always go back to accessor methods.

@oxinabox
Copy link
Contributor

oxinabox commented Jul 28, 2019

ok, I am down with exploring it and trying it out.

This looks fine, fix CI before merging yes?

@rofinn rofinn merged commit c3da500 into master Jul 30, 2019
@davidanthoff
Copy link
Contributor

@rofinn and @oxinabox could we tag and release a version here ASAP? Right now the whole released versions are broken because one gets the latest version of FilePathsBase, which removes functions like root, in combination with the latest FilePaths, which requires the root functions.

This is causing a major headache right now for a workshop that I have to teach tomorrow :) I'll also try to add the appropriate bounds to the registry now.

@rofinn
Copy link
Owner Author

rofinn commented Jul 30, 2019

Shouldn't FilePaths.jl be upperbounding FilePathsBase? Oh, damn, I forgot to tag a release after dropping the REQUIRE file. And the METADATA synching didn't place an upperbound.

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