-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
storage: unify storage/fs.FS and pebble/vfs.FS #98776
Conversation
0284348
to
78ad2a7
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.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @dt, @RaduBerinde, and @tbg)
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.
Thank you!
I'll defer to you on whether to merge this after the branch cut. There doesn't seem to be a pressing reason to merge it before, and there might be a flake or two due to the changed sorting behavior in List
.
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.
TFTRs!
Yeah, I'll sit on this until the branch cut.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @dt and @RaduBerinde)
counter-arg: backports will be easier for the next year+ of supporting 23.1 if we merge before branch cut. Can always revert if there's a flake. |
I'm fine either way, it is a long way to the release and hopefully bugs here would be obvious. |
The storage/fs.FS had largely the same interface as vfs.FS. The storage/fs.FS interface was intended as a temporary stepping stone to using pebble's vfs.FS interface throughout Cockroach for all filesystem access. This commit unifies the two. Epic: None Release note: None
TFTRs! bors r+ |
Build succeeded: |
The storage/fs.FS had largely the same interface as vfs.FS. The storage/fs.FS interface was intended as a temporary stepping stone to using pebble's vfs.FS interface throughout Cockroach for all filesystem access. This commit unifies the two.
Epic: None
Release note: None