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

Updating interface methods #470

Merged
merged 14 commits into from Nov 27, 2020
Merged

Updating interface methods #470

merged 14 commits into from Nov 27, 2020

Conversation

ramtinms
Copy link
Member

@ramtinms ramtinms commented Nov 25, 2020 •
edited

Description

This PR

  • updates several methods of the Interface and appends error return values, which enables the environment to return non-fatal errors (e.g. method not supported) as an outcome of calling the method.
  • introduces a new interface method (SetComputationUsed) which will be used by the runtime to report the computation used values

For contributor use:

  • Targeted PR against master branch
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work
  • Code follows the standards mentioned here
  • Updated relevant documentation
  • Re-reviewed Files changed in the Github PR explorer
  • Added appropriate labels

@ramtinms ramtinms changed the title [WIP] Adding errors to the interface Adding more errors to the interface methods Nov 25, 2020
@ramtinms ramtinms marked this pull request as ready for review November 25, 2020 23:03
@ramtinms ramtinms changed the title Adding more errors to the interface methods Updating interface methods Nov 25, 2020
turbolent
turbolent reviewed Nov 27, 2020
View reviewed changes
Copy link
Member

@turbolent turbolent 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, thanks for cleaning this up @ramtinms! Just a few comments

runtime/runtime.go Outdated Show resolved Hide resolved
runtime/runtime.go Outdated Show resolved Hide resolved
runtime/runtime.go Outdated Show resolved Hide resolved
runtime/runtime.go Outdated Show resolved Hide resolved
runtime/runtime.go Outdated Show resolved Hide resolved
runtime/runtime.go Outdated Show resolved Hide resolved
runtime/stdlib/crypto.go Outdated Show resolved Hide resolved
runtime/stdlib/crypto.go Outdated Show resolved Hide resolved
turbolent
turbolent approved these changes Nov 27, 2020
View reviewed changes
Copy link
Member

@turbolent turbolent left a comment

Choose a reason for hiding this comment

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

Nice, looks good!

@turbolent
Copy link
Member

@ramtinms OK if I rebase on master and merge?

janezpodhostnik
janezpodhostnik approved these changes Nov 27, 2020
View reviewed changes
Copy link
Contributor

@janezpodhostnik janezpodhostnik left a comment

Choose a reason for hiding this comment

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

👍

runtime/runtime.go Outdated Show resolved Hide resolved
runtime/runtime.go Outdated Show resolved Hide resolved
@turbolent turbolent force-pushed the ramtin/adding-errors-to-the-interface branch from 4c07fc2 to d37c9f3 Compare November 27, 2020 23:14
@turbolent turbolent merged commit 193cb51 into master Nov 27, 2020
@turbolent turbolent deleted the ramtin/adding-errors-to-the-interface branch November 27, 2020 23:18
@turbolent turbolent mentioned this pull request Dec 3, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@turbolent turbolent turbolent approved these changes

@janezpodhostnik janezpodhostnik janezpodhostnik approved these changes

@m4ksio m4ksio Awaiting requested review from m4ksio

@psiemens psiemens Awaiting requested review from psiemens

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ramtinms @turbolent @janezpodhostnik