-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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
Clean up some code related to file checksums #6861
Conversation
Summary: * Add missing unit test for schema stability of FileChecksumGenCrc32c (previously was only comparing to itself) * A lot of clarifying comments * Add some assertions for preconditions * Rename WritableFileWriter::CalculateFileChecksum -> UpdateFileChecksum * Simplify FileChecksumGenCrc32c with shared functions * Implement EndianSwapValue to replace unused EndianTransform Test Plan: new unit test passes before & after other changes
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.
@pdillinger 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks for making the changes!
@pdillinger has updated the pull request. Re-import the pull request |
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.
@pdillinger has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
@pdillinger merged this pull request in c7aedf1. |
Summary: * Add missing unit test for schema stability of FileChecksumGenCrc32c (previously was only comparing to itself) * A lot of clarifying comments * Add some assertions for preconditions * Rename WritableFileWriter::CalculateFileChecksum -> UpdateFileChecksum * Simplify FileChecksumGenCrc32c with shared functions * Implement EndianSwapValue to replace unused EndianTransform And incidentally since I had trouble with 'make check-format' GitHub action disagreeing with local run, * Output full diagnostic information when 'make check-format' fails in CI Pull Request resolved: facebook/rocksdb#6861 Test Plan: new unit test passes before & after other changes Reviewed By: zhichao-cao Differential Revision: D21667115 Pulled By: pdillinger fbshipit-source-id: 6a99970f87605aa024fa540c78cd519ff322c3e6 Signed-off-by: Changlong Chen <levisonchen@live.cn>
Summary: * Add missing unit test for schema stability of FileChecksumGenCrc32c (previously was only comparing to itself) * A lot of clarifying comments * Add some assertions for preconditions * Rename WritableFileWriter::CalculateFileChecksum -> UpdateFileChecksum * Simplify FileChecksumGenCrc32c with shared functions * Implement EndianSwapValue to replace unused EndianTransform And incidentally since I had trouble with 'make check-format' GitHub action disagreeing with local run, * Output full diagnostic information when 'make check-format' fails in CI Pull Request resolved: facebook/rocksdb#6861 Test Plan: new unit test passes before & after other changes Reviewed By: zhichao-cao Differential Revision: D21667115 Pulled By: pdillinger fbshipit-source-id: 6a99970f87605aa024fa540c78cd519ff322c3e6 Signed-off-by: Changlong Chen <levisonchen@live.cn>
Summary: * Add missing unit test for schema stability of FileChecksumGenCrc32c (previously was only comparing to itself) * A lot of clarifying comments * Add some assertions for preconditions * Rename WritableFileWriter::CalculateFileChecksum -> UpdateFileChecksum * Simplify FileChecksumGenCrc32c with shared functions * Implement EndianSwapValue to replace unused EndianTransform And incidentally since I had trouble with 'make check-format' GitHub action disagreeing with local run, * Output full diagnostic information when 'make check-format' fails in CI Pull Request resolved: facebook/rocksdb#6861 Test Plan: new unit test passes before & after other changes Reviewed By: zhichao-cao Differential Revision: D21667115 Pulled By: pdillinger fbshipit-source-id: 6a99970f87605aa024fa540c78cd519ff322c3e6 Signed-off-by: Changlong Chen <levisonchen@live.cn>
Summary: * Add missing unit test for schema stability of FileChecksumGenCrc32c (previously was only comparing to itself) * A lot of clarifying comments * Add some assertions for preconditions * Rename WritableFileWriter::CalculateFileChecksum -> UpdateFileChecksum * Simplify FileChecksumGenCrc32c with shared functions * Implement EndianSwapValue to replace unused EndianTransform And incidentally since I had trouble with 'make check-format' GitHub action disagreeing with local run, * Output full diagnostic information when 'make check-format' fails in CI Pull Request resolved: facebook/rocksdb#6861 Test Plan: new unit test passes before & after other changes Reviewed By: zhichao-cao Differential Revision: D21667115 Pulled By: pdillinger fbshipit-source-id: 6a99970f87605aa024fa540c78cd519ff322c3e6 Signed-off-by: Changlong Chen <levisonchen@live.cn>
Summary: * Add missing unit test for schema stability of FileChecksumGenCrc32c (previously was only comparing to itself) * A lot of clarifying comments * Add some assertions for preconditions * Rename WritableFileWriter::CalculateFileChecksum -> UpdateFileChecksum * Simplify FileChecksumGenCrc32c with shared functions * Implement EndianSwapValue to replace unused EndianTransform And incidentally since I had trouble with 'make check-format' GitHub action disagreeing with local run, * Output full diagnostic information when 'make check-format' fails in CI Pull Request resolved: facebook/rocksdb#6861 Test Plan: new unit test passes before & after other changes Reviewed By: zhichao-cao Differential Revision: D21667115 Pulled By: pdillinger fbshipit-source-id: 6a99970f87605aa024fa540c78cd519ff322c3e6 Signed-off-by: Changlong Chen <levisonchen@live.cn>
Summary: * Add missing unit test for schema stability of FileChecksumGenCrc32c (previously was only comparing to itself) * A lot of clarifying comments * Add some assertions for preconditions * Rename WritableFileWriter::CalculateFileChecksum -> UpdateFileChecksum * Simplify FileChecksumGenCrc32c with shared functions * Implement EndianSwapValue to replace unused EndianTransform And incidentally since I had trouble with 'make check-format' GitHub action disagreeing with local run, * Output full diagnostic information when 'make check-format' fails in CI Pull Request resolved: facebook/rocksdb#6861 Test Plan: new unit test passes before & after other changes Reviewed By: zhichao-cao Differential Revision: D21667115 Pulled By: pdillinger fbshipit-source-id: 6a99970f87605aa024fa540c78cd519ff322c3e6 Signed-off-by: Changlong Chen <levisonchen@live.cn>
Summary:
(previously was only comparing to itself)
And incidentally since I had trouble with 'make check-format' GitHub action disagreeing with local run,
Test Plan: new unit test passes before & after other changes