-
Notifications
You must be signed in to change notification settings - Fork 14
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
Add file error handling and structure handlers factory method #260
Conversation
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.
Added a few thoughts, but need to look through the changes more carefully when the suggestions have been processed.
I think we will have to add definitions of what error results mean for the state of the object that emitted the error. For instance, if a file operation fails, are subsequent operations allowed? Or in case a page pool fetch fails, can future get, flush, and close operations be trusted?
# Conflicts: # cpp/backend/common/file.cc # cpp/backend/common/file.h # cpp/backend/common/file_benchmark.cc # cpp/backend/common/file_test.cc # cpp/backend/common/page.h # cpp/backend/common/page_pool.h # cpp/backend/common/page_pool_benchmark.cc # cpp/backend/common/page_pool_test.cc # cpp/backend/index/file/index.h # cpp/backend/store/file/store.h
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.
Started the review, and worked through the file.* changes so fare and submitting those changes since I noticed a new version being available.
# Conflicts: # cpp/backend/common/BUILD # cpp/backend/depot/BUILD # cpp/backend/index/BUILD # cpp/backend/index/leveldb/BUILD # cpp/backend/store/BUILD # cpp/common/BUILD # cpp/common/status_util.h # cpp/common/status_util_test.cc # cpp/state/BUILD
Added list of things that needs to be done according to this PR. |
No description provided.