-
Notifications
You must be signed in to change notification settings - Fork 5
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
231 pass and store timestamp within record #232
Conversation
# Conflicts: # CHANGELOG.md # src/record/record.rs # src/storage/core.rs # tests/tests.rs
…Now they both returns latest record + stops on the first error
src/blob/core.rs
Outdated
@@ -262,28 +262,22 @@ where | |||
Ok(header) | |||
} | |||
|
|||
pub(crate) async fn read_last( | |||
#[allow(dead_code)] |
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.
Why this allow is needed?
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.
read_latest
is no longer used. Since this request Storage
uses get_latest_entry
. I thought it might be worth keeping this function for the future. What do you think?
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.
I think it's better to just keep in git history since it can make it harder to refactor code
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.
I've removed this function
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.
What about other dead_code allowances, they should be there?
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.
If we assume that the only way to read data now is by getting Entry
first, then we should keep read_all_entries
.
contains
is questionable, but as long as it is implemented as a simple redirection to get_latest_entry
, we can keep it. It is good to have such function just as a declaration, that the struct supports fast checking for existence.
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.
I removed all dead code
# Conflicts: # CHANGELOG.md # src/blob/core.rs # src/blob/entry.rs # src/storage/core.rs
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
Closes #231
Closes #229
Closes #276
Closes #241