Skip to content

Update Loadable Plug-in section #86

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

Closed
wants to merge 5 commits into from

Conversation

mhmohona
Copy link
Contributor

Explanation

Update Loadable Plug-in section - expanded explanation and added flowchart for better understanding of the workflow.

Related issue:

What type of PR is this

/kind documentation

Proposed Changes

Copy link
Collaborator

alabulei1 commented May 31, 2023

Hello, I am a code review bot on flows.network. Here are my reviews of code commits in this PR.


In general, the patches focus on providing more detailed instructions for creating WasmEdge loadable plugins, adding options for the programming language to create plugins, fixing typos, and maintaining the consistency of the documentation formatting. The potential issues are relatively minor, such as grammar and formatting errors or ambiguous changes that do not provide a clear context. The most important finding is the addition of new programming language support for the WasmEdge Plugin SDK, such as Rust and C, which will increase the flexibility of creating plugins that suit developers' preferences. The reviewer should check the accuracy of the updated content against the project guidelines and standards to ensure that the changes are aligned with the project's objectives.

Details

Commit b2fa4ed624114e749eb081ba6a6626b72fd1d40a

Key changes:

  • The introduction to Loadable Plug-ins has been modified to provide a more detailed explanation of shared libraries, how they can be used with WasmEdge, and the use of the WasmEdge SDK to create plugins.
  • A flowchart showing the basic steps involved in creating and using a Loadable Plug-in has been added.
  • The section on currently released plugins remains unchanged.

Potential problems:

  • None identified. However, further review of the modified text may reveal minor issues, such as grammar or formatting errors.

Commit 5ba52966872d56967f5e02e4e0fac51a03390fab

The key changes in this patch are the addition of more detailed steps for using a loadable plugin and how it can be registered with the WasmEdge runtime environment. It also mentions how a WebAssembly module can import the plugin module and execute its functions.

A potential problem could be if the patch does not provide enough context around the changes. It would be helpful to know why these changes were made and what problem they are trying to solve. Additionally, some developers may prefer more detailed instructions on how to use the API functions mentioned.

Commit aa1955cb88a8c3c7c40780a59af9988289276b0c

The key change in this GitHub patch is that it adds Rust as an option for developers to use the WasmEdge Plugin SDK when creating a loadable plugin for WasmEdge. Additionally, it clarifies the steps involved in creating and using a loadable plugin.

There are no apparent potential problems with this patch.

Commit 9b96815a4274e3cfbe8b26b77432a0f881a76fd9

Key changes:

  • The patch updates the section in the docs/contribute/plugin/intro.md file about loadable plugins for the WasmEdge runtime environment.
  • The update adds C language support to the WasmEdge Plugin SDK along with Rust support.
  • It also fixes a typo by correcting a hyperlink in the example code.

Potential problems:

  • None of the existing content has been changed or removed. The patch is a simple update that adds C language support, which should not affect the existing implementation.
  • However, the reviewer should verify the correctness of the updated content and ensure that the added information is accurate and complete. They should also check if the updated content aligns with the project's guidelines and standards.

Commit 6e337223b2a2d6c6f2e5b017c9c3691757837ba7

The patch is a minor change that removes a single whitespace character in the Loadable Plug-in documentation. There are no potential problems identified with this patch.

@alabulei1
Copy link
Collaborator

@q82419 Could you please helo review this?

@adithyaakrishna
Copy link
Contributor

@mhmohona Could you please fix the DCO check?

Copy link
Collaborator

@q82419 q82419 left a comment

Choose a reason for hiding this comment

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

You lost the most important part.
A plug-in can be loaded only after loading that plug-in from file with APIs.
Refer to the steps:
https://wasmedge.org/book/en/sdk/c/ref.html#plug-ins

Copy link
Contributor

@adithyaakrishna adithyaakrishna left a comment

Choose a reason for hiding this comment

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

@mhmohona Can we please make the above changes?

mhmohona added 3 commits June 8, 2023 09:40
"Signed-off-by: mhmohona <[email protected]>"
   Signed-off-by: mahfuza <[email protected]>

Signed-off-by: Mahfuza <[email protected]>
Signed-off-by: Mahfuza <[email protected]>
@mhmohona
Copy link
Contributor Author

mhmohona commented Jun 8, 2023

You lost the most important part. A plug-in can be loaded only after loading that plug-in from file with APIs. Refer to the steps: https://wasmedge.org/book/en/sdk/c/ref.html#plug-ins

Hello @q82419, thank you for your kind feedback. Regarding updating the section I have one confusion - shall I add these steps in the documents for plugin guideline?

@q82419
Copy link
Collaborator

q82419 commented Jun 9, 2023

You lost the most important part. A plug-in can be loaded only after loading that plug-in from file with APIs. Refer to the steps: https://wasmedge.org/book/en/sdk/c/ref.html#plug-ins

Hello @q82419, thank you for your kind feedback. Regarding updating the section I have one confusion - shall I add these steps in the documents for plugin guideline?

You should add these steps.
After loading the plugins (by all default paths or a given plugin path), you can then get the plugin context to create the module instance.

@alabulei1
Copy link
Collaborator

Please fix the DCO test and then we can merge this PR. Thanks.

@mhmohona
Copy link
Contributor Author

Reopened another PR to pass DOC test - #106

@mhmohona mhmohona closed this Jun 13, 2023
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.

4 participants