Skip to content

update loadable plugin section #106

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 4 commits into from
Closed

Conversation

mhmohona
Copy link
Contributor

Explanation

Update Loadable Plug-in section - expanded explanation and added flowchart for better understanding of the workflow. Also updated as per Yi-Ying's review.

Reopening PR #86 for passing DOC test

Related issue:

What type of PR is this

/kind documentation

Proposed Changes

Copy link
Collaborator

alabulei1 commented Jun 13, 2023

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


Overall Summary:

This pull request aims to update the loadable plugin section of the documentation by providing information on creating and loading plugins in the WasmEdge runtime environment. The key changes include adding explanations on creating loadable plugins, providing example code, and introducing a flowchart to illustrate the plugin loading process.

The potential issues and errors identified include the lack of explicit error handling in the example code for loading plugins and the need for more detailed explanations in the flowchart. It is important to mention the possibility of handling loading failures and improve the flowchart to cover alternative paths and error handling scenarios.

In terms of importance, ensuring proper error handling and clear flowchart explanations are crucial. These improvements will enhance the user experience and understanding of the plugin loading process.

Additionally, it's advisable to check if the flowchart renders correctly in the intended platform before merging the pull request. Furthermore, it is also recommended to review whether the code implementation was modified correctly and adequately, as the changes seem to be related to documentation formatting.

Overall, this pull request brings valuable updates to the loadable plugin section, with potential improvements to error handling and flowchart explanations.

Details

Commit 3a3a4405ef306f7489f4454e77a376571d6841ed

Key changes:

  • Added information about loadable plugins in the WasmEdge runtime environment.
  • Explained how to create loadable plugins using the WasmEdge Plugin SDK.
  • Provided example code for creating a simple loadable plugin.
  • Added information on how to load plugins from specific paths.
  • Included a flowchart illustrating the basic steps involved in creating and using a loadable plugin.

Potential problems:

  • There is no explicit error handling in the provided example code for loading plugins. It would be helpful to mention the possibility of error handling and handling potential loading failures.
  • The flowchart could be improved by providing more detailed explanations of each step.

Commit 503219050511f724638a205ba404ae4be67fec2f

Key Changes:

  • The section title "Creating Loadable Plug-in" has been added to make the purpose of the section clear.
  • The description of how to load plugins from default paths has been updated and improved.
  • A flowchart has been added to illustrate the plugin loading process.

Potential Problems:

  • The flowchart in the intro.md file is displayed using the Mermaid syntax, which may not be rendered properly in all documentation platforms or text editors. It would be best to check if the flowchart renders correctly in the intended platform before merging this pull request.
  • The flowchart does not provide any alternative paths or error handling for situations where the plugin is not found or fails to load. It may be beneficial to include such cases in the flowchart to provide a complete understanding of the plugin loading process.
  • The changes made in this pull request seem to be related to the documentation, so it would be advisable to review whether the actual code implementation has been modified correctly and adequately.

Commit 72d95acf74540e37999121f6adde8643b5120f06

Key Changes:

  • The format of the documentation in intro.md has been updated.
  • The steps for loading loadable plugins are now more clearly explained.

Potential Problems:

  • No potential problems were found in this patch.

Commit 74791e76162f8b2dd7a4a9eb01cc326bc9a6cee6

Key Changes:

  • Updated the format and indentation of the code block.

Potential Problems:

  • No potential problems were identified, as the changes are cosmetic and do not introduce any functional or logical issues.

@adithyaakrishna
Copy link
Contributor

@alabulei1 CI Workflows need your approval to run. Its a default feature of GH Actions. More Info: https://docs.github.com/en/actions/managing-workflow-runs/approving-workflow-runs-from-public-forks

@adithyaakrishna
Copy link
Contributor

@mhmohona Lint check failing here 👀

@mhmohona
Copy link
Contributor Author

@adithyaakrishna yes, its the same error I am getting all my PRs. I am unable to fix it. :3

@adithyaakrishna
Copy link
Contributor

@mhmohona There seems to be linting issues in many files which were not changed, I have created a PR #109 to fix the CI and linting issues :)

@alabulei1
Copy link
Collaborator

Hi @mhmohona

Still have one CI that didn't pass. Could you fix again?

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

mhmohona commented Jul 6, 2023

@alabulei1 I hope cli check will pass now.

@adithyaakrishna
Copy link
Contributor

@alabulei1 The workflows need to be approved manually for it to run 😶

Signed-off-by: Mahfuza <[email protected]>
@alabulei1
Copy link
Collaborator

@mhmohona The lint CI passed! But the DCO tests failed. Could you please take a look again? Thanks.

@mhmohona mhmohona closed this Jul 12, 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.

3 participants