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.

Sign up for GitHub

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

Jump to bottom

Allow passing label to node_create #811

Merged
merged 1 commit into from
Apr 16, 2020
Merged

Allow passing label to node_create #811

merged 1 commit into from
Apr 16, 2020

Conversation

tiziano88
Copy link
Collaborator

@tiziano88 tiziano88 commented Apr 2, 2020 •
edited
Loading

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.

@tiziano88 tiziano88 force-pushed the tzn_create_node_label branch 2 times, most recently from 92586d5 to 97f38c1 Compare April 2, 2020 22:28
@tiziano88 tiziano88 force-pushed the tzn_create_node_label branch from 97f38c1 to 6af6f31 Compare April 2, 2020 22:31
@tiziano88 tiziano88 changed the title Add support for passing label to node_create Allow passing label to node_create Apr 2, 2020
@tiziano88 tiziano88 force-pushed the tzn_create_node_label branch from 6af6f31 to 7321a09 Compare April 2, 2020 22:36
conradgrobler
conradgrobler approved these changes Apr 3, 2020
View reviewed changes
Copy link
Collaborator

@conradgrobler conradgrobler left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM

daviddrysdale
daviddrysdale reviewed Apr 3, 2020
View reviewed changes
Copy link
Contributor

@daviddrysdale daviddrysdale left a comment

Choose a reason for hiding this comment

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

LGTM so far, but presumably this needs either a corresponding change to the C++ runtime, or to be blocked until there's only a Rust runtime?

@tiziano88 tiziano88 added the blocked This pull request is blocked by another pull request or issue label Apr 3, 2020
@tiziano88
Copy link
Collaborator Author

Yes, the tests pass but the examples do not work because the C++ runtime does not implement the corresponding ABI functionality now. I am happy to wait until we switch to the Rust version of the Runtime in the C++ loader (#801), that should allow the examples to run successfully with this PR.

@tiziano88 tiziano88 force-pushed the tzn_create_node_label branch from 7321a09 to d93ed75 Compare April 16, 2020 10:07
@tiziano88 tiziano88 removed the blocked This pull request is blocked by another pull request or issue label Apr 16, 2020
@tiziano88 tiziano88 marked this pull request as ready for review April 16, 2020 10:16
@tiziano88 tiziano88 merged commit a2c48fe into project-oak:master Apr 16, 2020
@tiziano88 tiziano88 deleted the tzn_create_node_label branch April 16, 2020 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daviddrysdale daviddrysdale daviddrysdale approved these changes

@conradgrobler conradgrobler conradgrobler approved these changes

Assignees
No one assigned
Labels
cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@tiziano88 @daviddrysdale @conradgrobler @googlebot