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

Fixing #40: Support KeyValue #43

Open
wants to merge 26 commits into
base: master
Choose a base branch
from
Open

Conversation

mutlusun
Copy link
Contributor

Hello,

This PR is intended to fix #40 by adding support for the output of keyvalue frames. In addition, there are some small code cleanups. For example: I have removed checking the number of frames in frame.rs::new. I think there is no case in protobuf where a frame has two different contents and it is only necessary to check for unknown frame contents.

Best,
mutlusun

Copy link
Owner

@pajowu pajowu left a comment

Choose a reason for hiding this comment

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

Looks good. Howevery I want to keep the frame counting thing, because that might be the only mechanism we have to find new frames without weird crashes. I wrote a bit on that in the inline comment, I hope that makes sense?

src/frame.rs Show resolved Hide resolved
@mutlusun
Copy link
Contributor Author

Dear @pajowu ,

Thanks for your feedback. Please see my reply in the inline comment.

@pajowu
Copy link
Owner

pajowu commented Apr 30, 2021

Sorry for the long wait, I added another comment to our inline discussion

@mutlusun
Copy link
Contributor Author

mutlusun commented May 7, 2021

Thanks for your reply. If CI does not throw any error, it's ready to be merged from my side.

@mutlusun
Copy link
Contributor Author

mutlusun commented May 7, 2021

I'm sorry that the commit history got so messy ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support KeyValue, new SharedPreference-fields
2 participants