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

oak_runtime: Use prost instead of protobuf-rust #683

Merged
merged 1 commit into from
Mar 9, 2020

Conversation

blaxill
Copy link
Contributor

@blaxill blaxill commented Mar 6, 2020

Switch oak_runtime to prost in preparation for no_std support.

Checklist

  • Pull request affects core Oak functionality (e.g. runtime, SDK, ABI)
    • I have written tests that cover the code changes.
    • I have checked that these tests are run by Cloudbuild
    • I have updated documentation accordingly.
    • I have raised an issue to
      cover any TODOs and/or unfinished work.
  • Pull request includes prototype/experimental work that is under
    construction.

@blaxill blaxill added this to In progress in planning via automation Mar 6, 2020
@@ -26,7 +26,7 @@ fn test_app_config() {
);
let got = format!("{:?}", cfg);
Copy link
Contributor

Choose a reason for hiding this comment

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

Does prost have a way of generating text format protobufs? (ie. could we replace this with let got = cfg.to_text() and leave all the asserts as-is?)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah relying on debug strings does not seem ideal. In fact, why even test via a textual representation at all? It seems the assert could compare against an actual expected struct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed tests to structs.

@@ -26,7 +26,7 @@ fn test_app_config() {
);
let got = format!("{:?}", cfg);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah relying on debug strings does not seem ideal. In fact, why even test via a textual representation at all? It seems the assert could compare against an actual expected struct?

@blaxill blaxill merged commit f34c87f into project-oak:master Mar 9, 2020
planning automation moved this from In progress to Done Mar 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
planning
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants