-
Notifications
You must be signed in to change notification settings - Fork 1.7k
clean up duplicate information in FileOpener trait #17956
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
base: main
Are you sure you want to change the base?
Conversation
d649a43
to
d5a0cd9
Compare
d5a0cd9
to
3a5216a
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.
Good cleaning, thank you.
I saw there are some API changes, so added the api-change label.
Do you think it's worth adding this to the upgrade guide? I think it's quite straightforward and will impact few people. So maybe not worth the noise? |
&self, | ||
_partition_index: usize, | ||
file_meta: FileMeta, | ||
file: PartitionedFile, |
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.
1 nit: we probably can name it as partition_file
to be the same format as other function params
file_meta: FileMeta, | ||
_file: PartitionedFile, | ||
) -> Result<FileOpenFuture> { | ||
fn open(&self, file: PartitionedFile) -> Result<FileOpenFuture> { |
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.
ditto
&self, | ||
partition_index: usize, | ||
file_meta: FileMeta, | ||
file: PartitionedFile, |
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.
ditto
Ever since #16014 we've been passing duplicate information in.
FileMeta
is created entirely fromPartitionedFile
. This simplifies APIs and removes an unnecessary struct.