Remove Legacy and Custom FileWrapper classes from header files #7851
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
anand1976
reviewed
Jan 21, 2021
table/#table_reader_bench.cc#
Outdated
| @@ -0,0 +1,346 @@ | |||
| // Copyright (c) 2011-present, Facebook, Inc. All rights reserved. | |||
There was a problem hiding this comment.
I guess this file should be removed from the PR?
|
LGTM. I think this can be merged once #7858 is merged. Any idea why the clang-analyze test is failing? |
And remove LegacyFileSystem from public consumption.
Make WinEnv into a CompositeEnv
Made all of the uses of the Legacy and Composite FileWrapper classes private. Since all Env objects have a FileSystem already, most of the Legacy wrapping was adding an extra layer of wrapping. The Composite classes were already "private" to the CompositeEnv and just needed relocated into a source file.
anand1976
approved these changes
Jan 27, 2021
There was a problem hiding this comment.
@mrambacher has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@mrambacher has updated the pull request. You must reimport the pull request before landing. |
There was a problem hiding this comment.
@mrambacher has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@mrambacher has updated the pull request. You must reimport the pull request before landing. |
There was a problem hiding this comment.
@mrambacher has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
There was a problem hiding this comment.
@mrambacher has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@mrambacher merged this pull request in 4a09d63. |
pdillinger
added a commit
to pdillinger/rocksdb
that referenced
this pull request
Mar 4, 2021
facebook#7851)" This reverts commit 4a09d63.
pdillinger
added a commit
to pdillinger/rocksdb
that referenced
this pull request
Mar 4, 2021
facebook#7851)" This reverts commit 4a09d63.
codingrhythm
pushed a commit
to SafetyCulture/rocksdb
that referenced
this pull request
Mar 5, 2021
…ook#7851) Summary: Removed the uses of the Legacy FileWrapper classes from the source code. The wrappers were creating an additional layer of indirection/wrapping, as the Env already has a FileSystem. Moved the Custom FileWrapper classes into the CustomEnv, as these classes are really for the private use the the CustomEnv class. Pull Request resolved: facebook#7851 Reviewed By: anand1976 Differential Revision: D26114816 Pulled By: mrambacher fbshipit-source-id: db32840e58d969d3a0fa6c25aaf13d6dcdc74150
levichen94
pushed a commit
to bytedance/terarkdb
that referenced
this pull request
Jun 18, 2021
Summary: Removed the uses of the Legacy FileWrapper classes from the source code. The wrappers were creating an additional layer of indirection/wrapping, as the Env already has a FileSystem. Moved the Custom FileWrapper classes into the CustomEnv, as these classes are really for the private use the the CustomEnv class. Pull Request resolved: facebook/rocksdb#7851 Reviewed By: anand1976 Differential Revision: D26114816 Pulled By: mrambacher fbshipit-source-id: db32840e58d969d3a0fa6c25aaf13d6dcdc74150 Signed-off-by: Changlong Chen <levisonchen@live.cn>
levichen94
pushed a commit
to bytedance/terarkdb
that referenced
this pull request
Jun 18, 2021
Summary: Removed the uses of the Legacy FileWrapper classes from the source code. The wrappers were creating an additional layer of indirection/wrapping, as the Env already has a FileSystem. Moved the Custom FileWrapper classes into the CustomEnv, as these classes are really for the private use the the CustomEnv class. Pull Request resolved: facebook/rocksdb#7851 Reviewed By: anand1976 Differential Revision: D26114816 Pulled By: mrambacher fbshipit-source-id: db32840e58d969d3a0fa6c25aaf13d6dcdc74150 Signed-off-by: Changlong Chen <levisonchen@live.cn>
levichen94
pushed a commit
to bytedance/terarkdb
that referenced
this pull request
Jun 18, 2021
Summary: Removed the uses of the Legacy FileWrapper classes from the source code. The wrappers were creating an additional layer of indirection/wrapping, as the Env already has a FileSystem. Moved the Custom FileWrapper classes into the CustomEnv, as these classes are really for the private use the the CustomEnv class. Pull Request resolved: facebook/rocksdb#7851 Reviewed By: anand1976 Differential Revision: D26114816 Pulled By: mrambacher fbshipit-source-id: db32840e58d969d3a0fa6c25aaf13d6dcdc74150 Signed-off-by: Changlong Chen <levisonchen@live.cn>
mm304321141
pushed a commit
to bytedance/terarkdb
that referenced
this pull request
Jun 23, 2021
Summary: Removed the uses of the Legacy FileWrapper classes from the source code. The wrappers were creating an additional layer of indirection/wrapping, as the Env already has a FileSystem. Moved the Custom FileWrapper classes into the CustomEnv, as these classes are really for the private use the the CustomEnv class. Pull Request resolved: facebook/rocksdb#7851 Reviewed By: anand1976 Differential Revision: D26114816 Pulled By: mrambacher fbshipit-source-id: db32840e58d969d3a0fa6c25aaf13d6dcdc74150 Signed-off-by: Changlong Chen <levisonchen@live.cn>
mm304321141
pushed a commit
to bytedance/terarkdb
that referenced
this pull request
Jun 23, 2021
Summary: Removed the uses of the Legacy FileWrapper classes from the source code. The wrappers were creating an additional layer of indirection/wrapping, as the Env already has a FileSystem. Moved the Custom FileWrapper classes into the CustomEnv, as these classes are really for the private use the the CustomEnv class. Pull Request resolved: facebook/rocksdb#7851 Reviewed By: anand1976 Differential Revision: D26114816 Pulled By: mrambacher fbshipit-source-id: db32840e58d969d3a0fa6c25aaf13d6dcdc74150 Signed-off-by: Changlong Chen <levisonchen@live.cn>
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.
Removed the uses of the Legacy FileWrapper classes from the source code. The wrappers were creating an additional layer of indirection/wrapping, as the Env already has a FileSystem.
Moved the Custom FileWrapper classes into the CustomEnv, as these classes are really for the private use the the CustomEnv class.