Reduce code duplication in FileAccess #92167
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
BlueCube3310
force-pushed
the
file-access-the-final-season-part3-ep2
branch
from
b289773 to
71bce19
Compare
lawnjelly
approved these changes
Jun 8, 2024
There was a problem hiding this comment.
This looks great from a quick look.
The FileAccess classes were a mess and moving everything possible to base classes, and simplifying the derived classes makes sense imo (providing there are no downsides in some of the more esoteric versions like zip).
Could do with a secondary review and obviously lots of testing.
This is something I would like to do in 3.x too, along with adding caching in the wrapper, which speeds up loading by 2-3x in practice (OS caching doesn't appear very good in my tests when I last looked at this).
m4gr3d
approved these changes
Jun 8, 2024
|
Should be good to merge soon for 4.4. I'd suggest a rebase for good measure though. |
BlueCube3310
force-pushed
the
file-access-the-final-season-part3-ep2
branch
from
71bce19 to
205a10e
Compare
|
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#84107 added a lot of duplicate code to FileAccess classes. This PR moves most of that into the base FileAccess.
The child classes now only have to override the
get_bufferandstore_buffervirtual methods to function properly, while the rest is handled by the base class.This needs testing on all platforms, but it should work without issues.