Skip to content
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

Session Commits Persistence Implementation #98

Merged
merged 8 commits into from Oct 14, 2022
Merged

Conversation

miloszm
Copy link
Contributor

@miloszm miloszm commented Oct 13, 2022

Session Commits Persistence Implementation

Solves issue #89

@miloszm miloszm self-assigned this Oct 13, 2022
@miloszm miloszm added mark:next Strategic issues related to next versions of Testnet and mid/long term plans team:Core Low Level Core Development Team (Rust) type:feature implementing a new feature labels Oct 13, 2022
@miloszm miloszm marked this pull request as ready for review October 13, 2022 12:10
@miloszm miloszm requested review from krl and ureeves and removed request for ureeves October 13, 2022 12:11
Copy link
Member

@ureeves ureeves left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, one nit

vmx/src/commit.rs Show resolved Hide resolved
vmx/src/commit.rs Show resolved Hide resolved
@herr-seppia herr-seppia removed the mark:next Strategic issues related to next versions of Testnet and mid/long term plans label Oct 14, 2022
@@ -147,6 +197,46 @@ impl SessionCommits {
None => Err(SessionError("unknown session commit id".into())),
}
}

pub fn read<P: AsRef<Path>>(path: P) -> Result<Self, Error> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

naming here, using "read/write" is very generic and overlaps with the Read and Write traits unnecessarily, consider store/restore or something similar (in all cases where read/write is used)

@miloszm miloszm merged commit a3d4d31 into vmx Oct 14, 2022
@miloszm miloszm deleted the session-commit-persistence branch October 14, 2022 10:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team:Core Low Level Core Development Team (Rust) type:feature implementing a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants