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

Move the creation of Runtime dot graph to a separate submodule #1025

Merged
merged 1 commit into from
May 26, 2020

Conversation

rbehjati
Copy link
Contributor

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.

@rbehjati
Copy link
Contributor Author

I though oak/server/rust/oak_runtime/src/runtime/mod.rs is getting too big. So I moved the stuff related to the creation of the dot graph to a separate submodule.
What do you think @daviddrysdale, @tiziano88?

@@ -0,0 +1,271 @@
use crate::runtime::{ChannelHalfDirection, NodeId, Runtime};
Copy link
Contributor

Choose a reason for hiding this comment

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

Need top-level license text, and a module comment would be nice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Added.


pub trait DotGraph {
fn graph(&self) -> String;

Copy link
Contributor

Choose a reason for hiding this comment

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

My personal preference would be to skip the blank lines in this trait definition, but up to you.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Fixed.

Copy link
Collaborator

@tiziano88 tiziano88 left a comment

Choose a reason for hiding this comment

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

Nice!

@rbehjati rbehjati marked this pull request as ready for review May 26, 2020 18:10
@rbehjati rbehjati merged commit e075a5a into project-oak:master May 26, 2020
@rbehjati rbehjati deleted the graph-refactoring branch May 26, 2020 18:20
@github-actions
Copy link

Reproducibility index:

decf268bc29fcca1d10c433d0b87835285b85fde961c7e154a701d62a6546cba  ./target/wasm32-unknown-unknown/release/abitest_0_frontend.wasm
b0841f70d32d423a83d731c0990546131ed394ace6a69a5fe0ca6fb06423999b  ./target/wasm32-unknown-unknown/release/abitest_1_backend.wasm
8d7981c0ed6ee8447160495121ab3acae4d5d28e1ce4c6163055593ea1c424d9  ./target/wasm32-unknown-unknown/release/aggregator.wasm
6d972065aede6b2e79c13ce73ff54e57ca2fe146a81610ae31df3ebfc8c55209  ./target/wasm32-unknown-unknown/release/chat.wasm
a6aee6a97cc394522ce4871879d8f7de66f218f43223d3a5ba5bcfc60541bdec  ./target/wasm32-unknown-unknown/release/hello_world.wasm
1134fc3e30193384b56b226db2a6915d6be694c7e1d7d15489f6f8516ce8bd0e  ./target/wasm32-unknown-unknown/release/machine_learning.wasm
5fa2081c5279512fbca000284cab4802a76e83164dd59543ad7da32545f30e17  ./target/wasm32-unknown-unknown/release/private_set_intersection.wasm
e814b30f662463c38b61916a51231f501217f780d472a1d48297a177a5b7d864  ./target/wasm32-unknown-unknown/release/running_average.wasm
be0395b448aac0885c99d3b8884c41eb2fde8c838843dafdfc45aa71bb6451d6  ./target/wasm32-unknown-unknown/release/translator.wasm
209a5f4ed21660dae7d3e784ef3e40f1e30d84713853d39084d01eef29c0e845  ./target/x86_64-unknown-linux-musl/release/oak_loader

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

Successfully merging this pull request may close these issues.

4 participants