Update plugin section - developer guide

[mhmohona]

Explanation

Added following steps for plugin guide -

Developing WasmEdge Plugins

• Requirements for developing a WasmEdge plugin
• Choosing a programming language
  • C
  • C++
  • Rust
• Setting up the development environment
• Creating a WasmEdge plugin project
• Defining the plugin API
• Implementing the plugin API
• Compiling the WasmEdge plugin

Related issue- #85

Fixes #119

What type of PR is this

/kind documentation

Proposed Changes

[alabulei1]

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

---

Overall summary:

The patch includes various documentation updates for the plugin development section of the developer guide. The changes include adding new files, updating descriptions, adding a flowchart, providing instructions for setting up the development environment, and adding example code. While most of the changes seem reasonable and straightforward, there are several potential issues and errors that need to be addressed.

The potential problems identified are as follows:

• Lack of code changes: the patch only includes documentation updates, while code changes are typically reviewed separately.
• Accessibility of the flowchart: consider adding alternative text or a textual description of the flowchart to make it accessible to readers with visual impairments.
• Change in color of the flowchart: if there is no clear reason for the change, it can be suggested to revert it to the original color.
• Lack of detail in enabling specific backends or components: the instructions for enabling specific backends or components could be more detailed.
• Lack of clarity in setting up the project directory: the instructions for setting up the project directory could be clearer.
• Missing steps in the flowchart: the flowchart is missing some steps, such as testing and releasing the plugin.
• Lack of context and explanation: provide more context and explanations for the changes made in the patch, as some changes lack clear justifications or reasoning.
• Incomplete modifications to .vscode/settings.json file: review the entire file to ensure there are no conflicts or unintentional modifications.
• Empty plugin_development.md file for Rust: add content to the file before merging the changes.
• Lack of explanations for file removals and additions: provide explanations or justifications for the removal of certain files and the addition of package-lock.json and package.json files.

The most important findings include issues related to accessibility, missing instructions, lacking context and explanations, and incomplete modifications. These issues should be addressed to ensure the clarity, accessibility, and completeness of the documentation.

Details

Commit 2da1beca1cf18735a5d218981a69680391e59677

Key changes:

1. Added a new file plugin_requirement.md under the docs/contribute/plugin directory, which provides instructions for developing a WasmEdge plugin.
2. Updated the description in the _category_.json file to reflect that we will learn how to develop the WasmEdge Plugin System.

Potential problems:

1. The patch does not include any code changes, only documentation updates. Code changes are typically reviewed in a separate pull request.

Commit 916f66afe59a835e480020bb74e2ef7f2e0c49d6

Key Changes:

• Updated the title of the section from "Requirements for developing WasmEdge plugin" to "Requirements for Developing WasmEdge Plugin".
• Added a flowchart illustrating the steps involved in developing a WasmEdge plugin.
• Changed the color of the flowchart to a different shade.

Potential Problems:

• Although the flowchart provides a visual representation of the development process, it may not be accessible to all readers, especially those with visual impairments. Consider adding alternative text or a textual description of the flowchart.
• The change in color of the flowchart may not have any significant impact, but it could be helpful to know the reason behind this change. If there is no clear reason, you can suggest reverting it to the original color.
• The commit message could be more descriptive and provide more context about the changes made.

Overall, the changes made in this patch seem to be reasonable and straightforward. However, addressing the potential problems mentioned above would improve the accessibility and clarity of the document.

Commit a8c5a3b66d4f59056ea5d93f5cf97c3fc4a92558

Key Changes:

• Added a flowchart illustrating the process of developing a WasmEdge plugin.
• Provided recommendations for developers regarding the choice of programming language for plugin development.
• Updated instructions for setting up the development environment, including installing WasmEdge, compilers, necessary tools and dependencies, and enabling specific backends or components.
• Added instructions for creating a WasmEdge plugin project, including setting up the project directory and adding configuration files.

Potential Problems:

• The flowchart is missing some steps, such as testing and releasing the plugin.
• The instructions for enabling specific backends or components could be more detailed.
• The instructions for setting up the project directory could be clearer.

Commit 75013d470a263cebd002f4378f4ffc7f11d04839

Key changes:

• Added more specific instructions for each programming language (C, C++, Rust).
• Included links to separate pages for detailed instructions.
• Added a recommendation to use the C API for plugin development.
• Added more information about the choice of programming language.

Potential problems:

• The patch does not introduce any potential problems. It seems to be a straightforward update to the plugin requirement section of the developer guide.

Commit bf2b6321da94086907ad82e23b99c9e14d08b1c1

Key changes:

• The sidebar_position metadata in the develop_plugin_c.md file has been changed from 3 to 4.
• The sidebar_position metadata in the develop_plugin_cpp.md file has been changed from 4 to 5.

Potential problems:

• There are no potential problems identified in this patch. The changes seem to be simple metadata updates to the sidebar positions in the documentation files.

Commit 52162091ba330d3fcd1d6003b835c4b7fea27336

Key Changes:

• Added a new section "Develop WasmEdge Plug-in in C API" with detailed instructions on how to develop a WasmEdge plugin in C.
• Added a new flowchart showing the steps involved in developing a WasmEdge plugin.
• Added instructions for setting up the development environment, creating a plugin project, and writing the plugin code.
• Added example code for implementing host functions in C.

Potential Problems:

• The patch includes both additions and deletions of files. It's important to ensure that the deleted files are no longer needed and that all relevant content has been properly migrated to the new files.
• The patch includes changes to multiple files. It's important to review and ensure that the changes are consistent and logically organized.
• The patch introduces new content without mentioning the reason behind the changes or any relevant context. It would be beneficial to provide a brief explanation of the motivation behind the changes.

Overall, the patch introduces new documentation for developing WasmEdge plugins in C. The changes seem to be well-structured and provide step-by-step instructions for developers. It would be important to review the deleted files, ensure that all relevant content has been migrated, and provide some context for the changes.

Commit 54def63170b865e05765d7098d2a8349d6b0c4a9

Key changes:

• Added "docwriter.style" setting in the ".vscode/settings.json" file.
• Updated the label in "category.json" files for "Develop Plugin in C" and "Develop Plugin in CPP".
• Added new "category.json" and "plugin_development.md" files for "Develop Plugin in Rust".

Potential problems:

• It seems that there is a file named ".vscode/settings.json" being modified, but the patch only shows the changes made to the file and not the complete content. It would be helpful to review the entire file to ensure there are no conflicts or unintentional modifications.
• The new "plugin_development.md" file for "Develop Plugin in Rust" does not include any content, which might need to be addressed before merging the changes.

Commit 19784d1c111ad773e706e16a92724b90e9ce9306

Key changes:

• The patch removes several files related to plugin development in C, C++, and Rust.
• It adds a new package-lock.json file and a new package.json file.

Potential problems:

• The patch removes several files related to plugin development in C, C++, and Rust without providing any explanation or justification. It would be helpful to include a comment or commit message explaining why these files are being removed.
• It is unclear why the package-lock.json and package.json files are being added. It would be helpful to include a comment or commit message explaining the purpose of these files.
• There may be other changes in the patch that are not mentioned in the summary, so it would be important to review all changes carefully.

Commit 87c93739703cfed81a15e89d71e9be6abc17adf4

Key changes in the patch:

1. In the .vscode/settings.json file, the "docwriter.style": "Auto-detect" line was removed.

   Potential problem: The removal of this line may affect the documentation writing style detection in Visual Studio Code.

2. In docs/contribute/plugin/develop_plugin_c.md:

   • The installation steps for LLVM, GCC, text editors, and debugging tools were removed.
   • The section on implementing host functions was updated with example code.

   Potential problem: The removal of installation instructions for necessary tools and dependencies may make it harder for new developers to set up the development environment.

3. In docs/contribute/plugin/develop_plugin_cpp.md:

   • The installation steps for LLVM, GCC, text editors, and debugging tools were removed.
   • The section on setting up the project was removed.
   • The section on implementing host functions was updated with example code.

   Potential problem: The removal of installation instructions and project setup steps may make it harder for new developers to start developing the plugin.

Potential problems:

1. It's unclear why the line "docwriter.style": "Auto-detect" was removed from .vscode/settings.json. Further clarification is needed to determine if this change was intentional or if it was accidentally removed.

2. The removal of installation instructions for necessary tools and dependencies in both the C and C++ developer guides may make it harder for new developers to set up the development environment. These instructions should be provided or at least referenced for the benefit of new contributors.

3. The removal of the project setup steps in the C++ developer guide may make it harder for new developers to get started with plugin development. The guide should include instructions on setting up the project.

4. It's not clear why the section on setting up the project was removed in the C++ developer guide. Further explanation is needed to understand the reasoning behind this change.

Commit e30cc245367d513a298e99cd5534535b33f9e105

The key changes in this patch are:

1. Fixed a typo in the _category_.json file, changing "we will learn how to develop the WasmEdge Plugin System." to "we will learn how to develop the plug-in of WasmEdge."
2. Updated the language in the develop_plugin_c.md file to make it more clear and concise.
3. Fixed a couple of minor typos in the develop_plugin_c.md file.
4. Updated the language in the develop_plugin_cpp.md file to make it more clear and concise.
5. Fixed a couple of minor typos in the develop_plugin_cpp.md file.

Potential problems:

1. The patch does not include any test cases or changes related to testing. It would be beneficial to include test cases to ensure the functionality of the plugin is not affected by future updates.
2. The patch does not include any documentation updates regarding the changes made in the code. It would be helpful to include documentation updates to provide clarity for potential users or developers.
3. It is unclear what other changes this patch is part of, as it is labeled as "[PATCH 10/20]". It would be useful to have more context or information about the other patches in the series to evaluate potential dependencies or conflicts.

Commit 2a4f5c06a853fc4c894b577f87bafe4b0a1b3125

Key changes in the patch:

• Fixed a typographical error in multiple places, changing "plugin" to "plug-in".
• Updated some wording for clarity in the section titles and content.

Potential problems:

• The patch doesn't introduce any functional changes, only minor fixes and updates.

Commit 36abeadc0d8c51ea443d65dee1d298bce469fc19

Key changes:

• Updated the description in the _category_.json file to correct grammar.
• Removed outdated information about specific plug-ins and their installation commands in the develop_plugin_c.md and develop_plugin_cpp.md files.
• Added a reference to the installation guide for WasmEdge in the develop_plugin_c.md and develop_plugin_cpp.md files.
• Removed links to specific backends in the develop_plugin_c.md and develop_plugin_cpp.md files.
• Updated the code snippet in the develop_plugin_cpp.md file to reflect changes in the project name and added an installation command for the plugin.

Commit c328d3d13208e49128472890b05e95e9dd3488d9

Key Changes:

• The formatting has been updated in the develop_plugin_c.md file. Some lines are now properly aligned.
• Minor changes have been made to the text, such as removing unnecessary spaces.
• The URLs in the develop_plugin_c.md file have been updated to use relative paths.

Potential Problems:

• The patch does not introduce any functional changes or new features in the code. It mainly focuses on formatting and minor text modifications.
• While the formatting changes improve readability, they might make the diff larger and more difficult to review, especially if the reviewer is not familiar with the codebase.

Overall, the changes seem to be minor and do not raise any major concerns. They are mostly focused on improving code readability and consistency.

Commit 5d5b2c65c50c3a39f24140c5f8666b24025046f2

Key changes:

• Added Rust as one of the supported languages for implementing plug-ins in the WasmEdge runtime.
• Updated the sentence mentioning the supported languages and plug-in implementation guides.

Potential problems:

• No potential problems were identified in this patch.

Commit 7289e7feed1dc5ee2356e1d772b22ece9a264ef9

Key changes:

• Updated the documentation for developing WasmEdge plug-ins in C API, C++ API, and Rust-based API.
• Added a flowchart showing the steps needed for developing WasmEdge plug-ins in Rust-based API.

Potential problems:

• In the C API section, the description incorrectly mentions C++ as the base API for registering extension modules and host functions.
• In the C++ API section, the description incorrectly mentions C as the base API for registering extension modules and host functions.
• In the Rust-based API section, the flowchart is missing a statement explaining the purpose of each step.
• The recommendation note in the Rust-based API section suggests using the C API for plug-in development without providing a clear reason for this recommendation.

Commit 816d5581189457f0292d16ea4462ecae79d4cb98

Key changes:

• In the docusaurus.config.js file, the markdown object was updated to include the mermaid property set to true.
• The themes array was updated to include @docusaurus/theme-mermaid.

Potential problems:

• The patch does not include any additional information or explanations for the changes made. It would be helpful to provide some context or reasoning behind these updates.
• There is a discrepancy between the number of insertions and deletions reported in the patch. The patch claims there are 5 insertions and 4 deletions, but the diff only shows 4 insertions and 3 deletions. This discrepancy should be clarified and resolved.

Commit 07e5fdc7e56ca7bbdb4f64417480908ab6d21b4c

Key changes in the patch are:

1. Renaming of the library from wasmedgePluginWasiLogging to wasmedgePluginTest.
2. Updating the target compile options to use the new library name.
3. Updating the target include directories to use the new library name.
4. Updating the target link libraries to use the new library name.
5. Updating the install target name to use the new library name.

Potential problems:

1. The patch changes the library name from wasmedgePluginWasiLogging to wasmedgePluginTest. Make sure that this change is intended and does not break any dependencies or usage of the plugin.
2. The patch updates the target compile options, include directories, link libraries, and install target for the plugin. Make sure that these changes are correct and do not introduce any issues or conflicts with other parts of the codebase.
3. The patch does not provide any explanation or justification for the changes made. It would be helpful to have some context or explanation about why these changes were necessary or beneficial.

Commit 5b703db963b39183a2ecb5c7be4d613bd589520b

Key Changes:

• The entire "Usages of WasmEdge Plug-in" section has been removed.
• The heading "Benefits of Using WasmEdge Plug-in" is now the first heading in the document.

Potential Problems:

• The entire "Usages of WasmEdge Plug-in" section has been removed without any replacement or explanation. This could result in a loss of important information and make the document less useful for readers.
• It is unclear why the section was removed and what motivated this change.
• It would be helpful to have a commit message with more context and explanation for this particular change.

Overall, the removal of the "Usages of WasmEdge Plug-in" section raises concerns about potential loss of information and incomplete documentation. It would be beneficial to either explain the reason for its removal or provide a replacement section with relevant information. Additionally, providing more context in the commit message would help reviewers understand the motivation behind this change.

Commit f3726db7546c85c137f78186e08718930b3caf99

The key changes in this patch include:

• Fixed a typo in the plugin section of the developer guide.
• Replaced occurrences of "plugin" with "plug-in" for consistency.
• Added a flowchart to illustrate the steps for developing a WasmEdge plug-in in C API.
• Added a flowchart to illustrate the steps for developing a WasmEdge plug-in in C++ API.
• Added a flowchart to illustrate the steps for developing a WasmEdge plug-in in Rust SDK.
• Updated the category description for the plugin section in the Chinese documentation.

Potential problems in this patch could include:

• The changes made only fix a typo and improve consistency, so there shouldn't be any major problems introduced.
• However, the changes in the .md files should be reviewed to ensure the content is accurate and up-to-date.
• The changes to the docusaurus.config.js file seem to enable the use of mermaid diagrams in markdown, but it's unclear if this change is necessary or intended in this pull request.
• It's important to verify that the flowcharts added accurately represent the steps for developing plug-ins in the respective programming languages.

Commit 20d85e4d0c11ef48813aba10e788960aa48fc9de

Key changes:

• Added configurations for the @easyops-cn/docusaurus-search-local plugin.
• Updated the themeConfig metadata, image, navbar, and footer configurations.
• Extended the config to include the mermaid feature for Markdown files.
• Added the @docusaurus/theme-mermaid theme.

Potential problems:

• The JSON syntax within the @easyops-cn/docusaurus-search-local plugin configuration is missing a closing parenthesis.
• The indentation within the themeConfig and docs sections is inconsistent.
• There is a typo in the comment at the end of the themeConfig section ("inlcude" instead of "include").
• It's unclear why the config is being extended to include the @docusaurus/theme-mermaid theme without any corresponding configuration changes.
• It would be helpful to add comments or explanations for the purpose of each configuration change.

Overall, the patch introduces some updates and extensions to the Docusaurus configuration file. However, there are some potential problems and areas that could use more clarity in terms of the changes made.

[alabulei1]

Hi @mhmohona

Thanks for this. Much appreciated.

Your PR contains several factual errors and cannot be merged before It is fixed. Please test your writing before submitting the PR for review.

[adithyaakrishna]

@mhmohona The lint might fail on this PR, @alabulei1 the workflows need to be manually approved to run for this PR, https://docs.github.com/en/actions/managing-workflow-runs/approving-workflow-runs-from-public-forks

[mhmohona]

Hello @alabulei1, thank you for your feedback!
Regarding installation, its a good point. I went through the previous doc and put info from there. I will reorganize the section accordingly and update it.

[q82419]

Please not use spaces for the file names.

[q82419]

Please use the related link when linking to the other doc pages.

[q82419]

This section is not correct.
The plug-in of WasmEdge only provides the portable host modules and host functions for WASM extension. We don't have such cases you mentioned yet.

[q82419]

Still not correct. Delete this section.

[q82419]

PLEASE DO NOT RESOLVE THIS WITHOUT REQUESTED MODIFICATION.

[q82419]

Still not correct. Delete this section.

[q82419]

@adithyaakrishna PTAL for the configs, thanks.

[alabulei1]

Hi @adithyaakrishna

Can you take a look again? Thanks.

[alabulei1]

Hi @mhmohona

Another CI fails. Please check it out!

[mhmohona]

It seems to fail because of docusaurus.config.js file. @adithyaakrishna kindly can you have a look at it?

[adithyaakrishna]

Will check it rn

[adithyaakrishna]

@mhmohona Can you please use the below config?

// @ts-check
// Note: type annotations allow type checking and IDEs autocompletion

const lightCodeTheme = require('prism-react-renderer/themes/nightOwlLight');
const darkCodeTheme = require('prism-react-renderer/themes/dracula');

require('dotenv').config();

/** @type {import('@docusaurus/types').Config} */
const config = {
    title: 'WasmEdge Developer Guides',
    tagline: 'Serverless functions anywhere in the cloud, in data / AI pipelines, in SaaS platforms, and on edge devices.',
    url: 'https://wasmedge.org/',
    baseUrl: '/docs/',
    onBrokenLinks: 'throw',
    onBrokenMarkdownLinks: 'warn',

    favicon: 'img/favicon.ico',

    // GitHub pages deployment config.
    // If you aren't using GitHub pages, you don't need these.
    organizationName: 'Second State', // Usually your GitHub org/user name.
    projectName: 'WasmEdge', // Usually your repo name.

    // Even if you don't use internalization, you can use this field to set useful
    // metadata like html lang. For example, if your site is Chinese, you may want
    // to replace "en" with "zh-Hans".
    i18n: {
        defaultLocale: 'en',
        locales: ['en', 'zh', 'zh-TW'],
        localeConfigs: {
            en: {
                label: 'English'
            },
            "zh": {
                label: '简体中文'
            }
        }
    },

    presets: [
        [
            'classic',
            /** @type {import('@docusaurus/preset-classic').Options} */
            ({
                docs: {
                    sidebarPath: require.resolve('./sidebars.js'),
                    remarkPlugins: [import('remark-deflist'), require('./env-plugin')],
                    // Please change this to your repo.
                    // Remove this to remove the "edit this page" links.
                    routeBasePath: '/',
                    editUrl:
                        'https://github.com/wasmedge/docs/blob/main/',
                },
                theme: {
                    customCss: require.resolve('./src/css/custom.css'),
                },
            }),
        ],
    ],

    themes: [
        [
            "@easyops-cn/docusaurus-search-local",
            /** @type {import("@easyops-cn/docusaurus-search-local").PluginOptions} */
            // @ts-ignore
            {
                docsRouteBasePath: '/',
                hashed: true,
                indexBlog: false,
                indexPages: true,
                language: ["en", "zh"],
                highlightSearchTermsOnTargetPage: true,
                explicitSearchResultPath: true,
                searchBarShortcut: true,
                searchBarShortcutHint: true,
                searchBarPosition: "right",
            },
        ],
    ],
    

    themeConfig:
        /** @type {import('@docusaurus/preset-classic').ThemeConfig} */
        ({
            metadata: [{ name: 'keywords', content: 'wasmedge, wasm, web assembly, rust, cncf, edge devices, cloud, serverless' }, { name: 'twitter:card', content: 'summary' }],
            image: "./static/img/wasm_logo.png",
            announcementBar: {
                id: "start",
                content:
                    '⭐️ If you like WasmEdge, give it a star on <a target="_blank" rel="noopener noreferrer" href="https://github.com/WasmEdge/WasmEdge">GitHub</a>! ⭐️',
            },
            navbar: {
                title: 'WasmEdge',
                logo: {
                    alt: 'WasmEdge Logo',
                    src: 'img/logo.svg',
                },
                items: [
                    {
                        type: 'doc',
                        docId: 'start/overview',
                        position: 'left',
                        label: 'Getting Started',
                    }, {
                        type: 'doc',
                        docId: 'develop/overview',
                        position: 'left',
                        label: 'Develop',
                    }, {
                        type: 'doc',
                        docId: 'embed/overview',
                        position: 'left',
                        label: 'Embed',
                    }, {
                        type: 'doc',
                        docId: 'contribute/overview',
                        position: 'left',
                        label: 'Extend',
                    }, {
                        type: 'localeDropdown',
                        position: 'right',
                        dropdownItemsBefore: [],
                        dropdownItemsAfter: [],
                        className: 'icon-link language navbar__item',
                    }, {
                        href: 'https://github.com/WasmEdge/WasmEdge',
                        className: "header-github-link",
                        position: 'right',
                    },
                ],
            },
            docs: {
                sidebar: {
                    hideable: true,
                },
            },
            footer: {
                logo: {
                    alt: 'WasmEdge logo',
                    src: '/img/wasmedge_logo.svg',
                    href: 'https://wasmedge.org/',
                },
                style: 'dark',
                links: [
                    {
                        title: 'Docs',
                        items: [
                            {
                                label: 'Getting Started',
                                to: '/start/overview',
                            }, {
                                label: 'Develop',
                                to: '/develop/overview',
                            }, {
                                label: 'Embeds',
                                to: '/embed/overview',
                            }, {
                                label: 'Contribute',
                                to: '/contribute/overview',
                            }
                        ],
                    }, {
                        title: 'Resources',
                        items: [
                            {
                                label: 'Github',
                                href: 'https://github.com/WasmEdge/WasmEdge',
                            }, {
                                label: 'Second State',
                                href: 'https://www.secondstate.io/',
                            }, {
                                label: 'Articles & Blog',
                                href: 'https://www.secondstate.io/articles/'
                            }, {
                                label: 'WasmEdge Talks',
                                to: '/talks'
                            }, {
                                label: 'Releases',
                                to: '/releases'
                            }
                        ],
                    },
                    {
                        title: 'Community',
                        items: [
                            {
                                label: 'Mailing List',
                                href: 'https://groups.google.com/g/wasmedge/'
                            }, {
                                label: 'Discord',
                                href: 'https://discord.gg/U4B5sFTkFc',
                            }, {
                                label: 'Twitter',
                                href: 'https://twitter.com/realwasmedge',
                            }, {
                                label: 'Slack #WasmEdge',
                                href: 'https://cloud-native.slack.com/archives/C0215BBK248'
                            }, {
                                label: 'Community Meeting',
                                href: 'https://docs.google.com/document/d/1iFlVl7R97Lze4RDykzElJGDjjWYDlkI8Rhf8g4dQ5Rk/edit?usp=sharing'
                            }
                        ],
                    },
                ],
                copyright: `Copyright © ${new Date().getFullYear()} WasmEdge. Built with Docusaurus. <br /> <a href="https://github.com/WasmEdge/docs/blob/main/CODE_OF_CONDUCT.md" target="_blank">Code of Conduct</a>`,
            },
            prism: {
                theme: lightCodeTheme,
                darkTheme: darkCodeTheme,
                additionalLanguages: ['rust', 'bash', 'typescript', 'csharp', 'lua', 'protobuf', 'powershell', 'toml', 'yaml', 'wasm', 'javascript', 'go'],
            },
        }),
};

// Extending config to inlcude mermaid also
const extendedConfig = {
    ...config,
    markdown: {
        mermaid: true,
    },
    themes: ['@docusaurus/theme-mermaid'],
};

module.exports = extendedConfig;

[adithyaakrishna]

SHIP IT! 🚀

[adithyaakrishna]

@mhmohona Could you please mention in the PR description that this PR also fixes #119

GitHub will auto link it if you just add Fixes #119

[alabulei1]

It looks that everything is good now! Thanks @mhmohona, @adithyaakrishna and @q82419 .

I'm going to merge this PR!