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

FIL VM Runtime Interface Update #1064

Merged
merged 9 commits into from
Sep 16, 2020
Merged

FIL VM Runtime Interface Update #1064

merged 9 commits into from
Sep 16, 2020

Conversation

yiannisbot
Copy link
Collaborator

This PR updates the specification for the FIL VM Runtime Interface & Exit codes.

I have excluded mention to the vm/implementation and code loading, as I found these parts too implementation-specific and therefore, not belonging to this spec. Let me know if we want to do otherwise and if so, which are the important points from those parts that we should be referring to.

Copy link
Member

@anorth anorth left a comment

Choose a reason for hiding this comment

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

I object to duplicating all this code and prose from the specs-actors repo. That repo is the spec, it was broken out from this one precisely to avoid the maintenance nightmare that will arise from having two versions of this, only one of which (the executable code) stands a chance of being correct.

Please link to the specs-actors repo instead. To the extent that prose can be improved, please do so with PRs to the specs-actors repo.

content/systems/filecoin_vm/runtime/gascost/_index.md Outdated Show resolved Hide resolved
@yiannisbot
Copy link
Collaborator Author

@anorth I've removed all redundant text and code and pointed to the respective repositories. Let me know if there's some part of the code that needs a special mention or description in text, so that I work to include it.

anorth
anorth previously approved these changes Sep 1, 2020
@yiannisbot
Copy link
Collaborator Author

@hugomrdias this is ready to merge.

@hugomrdias
Copy link
Contributor

still targeting beta

@yiannisbot yiannisbot added the hint: ready to merge Hint: PR is ready to be merged label Sep 1, 2020
@yiannisbot yiannisbot changed the base branch from beta to master September 3, 2020 09:52
@yiannisbot yiannisbot dismissed anorth’s stale review September 3, 2020 09:52

The base branch was changed.

content/systems/filecoin_vm/runtime/_index.md Outdated Show resolved Hide resolved
content/systems/filecoin_vm/runtime/_index.md Outdated Show resolved Hide resolved
content/systems/filecoin_vm/runtime/_index.md Outdated Show resolved Hide resolved
@hugomrdias hugomrdias removed the hint: ready to merge Hint: PR is ready to be merged label Sep 4, 2020
@hugomrdias
Copy link
Contributor

you didnt change the links yet

@yiannisbot
Copy link
Collaborator Author

@hugomrdias I did now :)

@daviddias daviddias added the scope: content Scope: Editing content of the spec label Sep 8, 2020
@hugomrdias hugomrdias removed the scope: content Scope: Editing content of the spec label Sep 16, 2020
@yiannisbot yiannisbot added the hint: ready to merge Hint: PR is ready to be merged label Sep 16, 2020
@hugomrdias hugomrdias merged commit e07b11c into master Sep 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hint: ready to merge Hint: PR is ready to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants