Skip to content

update fd_permissions_set() signature#213

Merged
pchickey merged 2 commits into
WebAssembly:masterfrom
cjihrig:patch-2
Feb 7, 2020
Merged

update fd_permissions_set() signature#213
pchickey merged 2 commits into
WebAssembly:masterfrom
cjihrig:patch-2

Conversation

@cjihrig

@cjihrig cjihrig commented Jan 26, 2020

Copy link
Copy Markdown
Contributor

It appears that the permissions argument to permissions_set() was labeled as an output instead of an input.

@kubkon kubkon left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hey! Thanks for the PR! It indeed looks like a bug. Thanks for fixing this!

@kubkon kubkon requested review from sbc100 and sunfishcode January 30, 2020 23:02
@kubkon

kubkon commented Jan 30, 2020

Copy link
Copy Markdown
Contributor

Oh, and BTW, in order to make the CI green whenever introducing a change to the *.witx files in a PR, you should regenerate the docs and include them in the PR. You can do this by running

$ pushd tools/witx
$ cargo run -- repo-docs
$ popd

This will regenerate the docs for all phases which you can then commit in the PR :-)

@cjihrig

cjihrig commented Jan 30, 2020

Copy link
Copy Markdown
Contributor Author

Thanks @kubkon. I noticed the link checking was failing, but also noticed that the witx and generated docs hadn't been kept in sync, so I wasn't sure if that was something I was supposed to do.

@kubkon

kubkon commented Jan 30, 2020

Copy link
Copy Markdown
Contributor

Thanks @kubkon. I noticed the link checking was failing, but also noticed that the witx and generated docs hadn't been kept in sync, so I wasn't sure if that was something I was supposed to do.

Yep, we got behind on keeping our docs up-to-date with the state of the *.witx files somewhat but that is now fixed in #217. However, even if the docs were kept up-to-date, since you have modified the *.witx here by replacing result with param in one of permissions_set's args, the CI would still be unhappy until you regenerated the docs with your change included :-)

It appears that the permissions argument to permissions_set()
was labeled as an output instead of an input.
@cjihrig

cjihrig commented Feb 4, 2020

Copy link
Copy Markdown
Contributor Author

Ping. I pushed a commit to regenerate the docs.

@pchickey pchickey merged commit 857e2e2 into WebAssembly:master Feb 7, 2020
@cjihrig cjihrig deleted the patch-2 branch February 7, 2020 03:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants