Skip to content

Conversation

@hugo-dc
Copy link
Member

@hugo-dc hugo-dc commented Dec 13, 2018

@hugo-dc hugo-dc requested review from axic and lrettig December 13, 2018 23:30
@hugo-dc hugo-dc changed the title [wip] Add documentation on Binaryen and Wabt Add documentation on Binaryen and Wabt Dec 14, 2018
Copy link
Member

@lrettig lrettig left a comment

Choose a reason for hiding this comment

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

Thanks @hugo-dc. A couple of high-level questions:

  1. Is this linked into the main README doc anywhere?
  2. Do you think it would be possible to add a little info at the beginning of this doc discussing the differences between binaryen and wabt? E.g., why do we need two of them? Is there some benefit to one over the other?

Copy link
Member

@lrettig lrettig left a comment

Choose a reason for hiding this comment

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

Made a few small tweaks and some suggestions. Thanks Hugo!

@lrettig
Copy link
Member

lrettig commented Dec 14, 2018

Instructions on installing cmake are already here in the main doc - so maybe you could link to that part inline here!

@hugo-dc
Copy link
Member Author

hugo-dc commented Dec 15, 2018

@lrettig Thanks a lot for reviewing and improving the document!, I have made some changes according to your comments.

Lane Rettig added 3 commits December 15, 2018 15:05
Copy link
Member

@lrettig lrettig left a comment

Choose a reason for hiding this comment

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

Just one final clarification! Thanks

@lrettig lrettig mentioned this pull request Dec 27, 2018
Change capitalization, reformat slightly, add a link
@lrettig
Copy link
Member

lrettig commented Dec 27, 2018

Thanks @hugo-dc! Made a few final tweaks, have a look. LGTM.

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