-
Notifications
You must be signed in to change notification settings - Fork 830
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
object_store: Add enabled-by-default "fs" feature #6636
base: main
Are you sure you want to change the base?
Conversation
/cc @tustvold |
I had a quick look and this looks plausible, however, it is a breaking change and will therefore need to wait for the next major release |
799e4cf
to
77c3931
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense to me. Thank you @Turbo87
Could you also please add a test for clippy here:
- name: Run clippy with default features |
Specifically these two combinations?
--no-deafult-features
--no-default-features --features fs
?
77c3931
to
5efbd80
Compare
5efbd80
to
0e496ac
Compare
@@ -70,6 +70,10 @@ | |||
//! | |||
//! Feature flags are used to enable support for other implementations: | |||
//! | |||
#![cfg_attr( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"local filesystem" should be removed from the list in the previous paragraph (GitHub doesn't let me add a comment there, but this one here:
arrow-rs/object_store/src/lib.rs
Line 69 in 13f3f21
//! * Local filesystem: [`LocalFileSystem`](local::LocalFileSystem) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch. fixed! :)
0e496ac
to
a48eb8b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Please delay the merge until 0.12 though (see #6596).
I hope to be able to merge early next week |
Which issue does this PR close?
Closes #6055.
Rationale for this change
see #6055 😉
What changes are included in this PR?
This PR introduces a new
fs
feature for theobject_store
crate, which is enabled by default. If disabled, thelocal
module is excluded from the crate and thewalkdir
dependency is skipped.Are there any user-facing changes?
Only for users of
default-features = false
, but since this crate did not have any default features before there probably won't be manyobject_store
users that specifydefault-features = false
.