-
Notifications
You must be signed in to change notification settings - Fork 59
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
Update plugin section - developer guide #102
Update plugin section - developer guide #102
Conversation
Signed-off-by: Mahfuza <[email protected]>
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:
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. DetailsCommit 2da1beca1cf18735a5d218981a69680391e59677Key changes:
Potential problems:
Commit 916f66afe59a835e480020bb74e2ef7f2e0c49d6Key Changes:
Potential Problems:
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 a8c5a3b66d4f59056ea5d93f5cf97c3fc4a92558Key Changes:
Potential Problems:
Commit 75013d470a263cebd002f4378f4ffc7f11d04839Key changes:
Potential problems:
Commit bf2b6321da94086907ad82e23b99c9e14d08b1c1Key changes:
Potential problems:
Commit 52162091ba330d3fcd1d6003b835c4b7fea27336Key Changes:
Potential Problems:
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 54def63170b865e05765d7098d2a8349d6b0c4a9Key changes:
Potential problems:
Commit 19784d1c111ad773e706e16a92724b90e9ce9306Key changes:
Potential problems:
Commit 87c93739703cfed81a15e89d71e9be6abc17adf4Key changes in the patch:
Potential problems:
Commit e30cc245367d513a298e99cd5534535b33f9e105The key changes in this patch are:
Potential problems:
Commit 2a4f5c06a853fc4c894b577f87bafe4b0a1b3125Key changes in the patch:
Potential problems:
Commit 36abeadc0d8c51ea443d65dee1d298bce469fc19Key changes:
Commit c328d3d13208e49128472890b05e95e9dd3488d9Key Changes:
Potential Problems:
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 5d5b2c65c50c3a39f24140c5f8666b24025046f2Key changes:
Potential problems:
Commit 7289e7feed1dc5ee2356e1d772b22ece9a264ef9Key changes:
Potential problems:
Commit 816d5581189457f0292d16ea4462ecae79d4cb98Key changes:
Potential problems:
Commit 07e5fdc7e56ca7bbdb4f64417480908ab6d21b4cKey changes in the patch are:
Potential problems:
Commit 5b703db963b39183a2ecb5c7be4d613bd589520bKey Changes:
Potential Problems:
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 f3726db7546c85c137f78186e08718930b3caf99The key changes in this patch include:
Potential problems in this patch could include:
Commit 20d85e4d0c11ef48813aba10e788960aa48fc9deKey changes:
Potential problems:
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. |
Signed-off-by: Mahfuza <[email protected]>
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. |
Signed-off-by: Mahfuza <[email protected]>
@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 |
Hello @alabulei1, thank you for your feedback! |
Signed-off-by: Mahfuza <[email protected]>
Signed-off-by: Mahfuza <[email protected]>
Signed-off-by: Mahfuza <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please not use spaces for the file names.
docs/contribute/plugin/Develop Plugin in C/plugin_development.md
Outdated
Show resolved
Hide resolved
docs/contribute/plugin/Develop Plugin in C/plugin_development.md
Outdated
Show resolved
Hide resolved
Signed-off-by: Mahfuza <[email protected]>
54def63
to
2a9dacd
Compare
…/mhmohona/WasmEdge_docs into requirements-for-WasmEdge-plugin
Signed-off-by: Mahfuza <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use the related link when linking to the other doc pages.
docs/contribute/plugin/intro.md
Outdated
In this diagram, the _Host Application_ represents the application or environment where the Wasmedge runtime is embedded or used. The _Plug-in Shared Library_ refers to the library containing the plug-in code and functions that extend the functionality of the Wasmedge runtime. The _Wasmedge Runtime_ represents the runtime environment that executes WebAssembly modules, including the core runtime and any registered plug-ins. | ||
|
||
## Usages of WasmEdge Plug-in | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still not correct. Delete this section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PLEASE DO NOT RESOLVE THIS WITHOUT REQUESTED MODIFICATION.
Signed-off-by: Mahfuza <[email protected]>
Signed-off-by: Mahfuza <[email protected]>
Signed-off-by: Mahfuza <[email protected]>
Signed-off-by: Mahfuza <[email protected]>
Signed-off-by: Mahfuza <[email protected]>
docs/contribute/plugin/intro.md
Outdated
In this diagram, the _Host Application_ represents the application or environment where the Wasmedge runtime is embedded or used. The _Plug-in Shared Library_ refers to the library containing the plug-in code and functions that extend the functionality of the Wasmedge runtime. The _Wasmedge Runtime_ represents the runtime environment that executes WebAssembly modules, including the core runtime and any registered plug-ins. | ||
|
||
## Usages of WasmEdge Plug-in | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still not correct. Delete this section.
Signed-off-by: Mahfuza <[email protected]>
Signed-off-by: Mahfuza <[email protected]>
@adithyaakrishna PTAL for the configs, thanks. |
Signed-off-by: Mahfuza <[email protected]>
Can you take a look again? Thanks. |
Hi @mhmohona Another CI fails. Please check it out! |
It seems to fail because of |
Will check it rn |
@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; |
Signed-off-by: Mahfuza <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SHIP IT! 🚀
It looks that everything is good now! Thanks @mhmohona, @adithyaakrishna and @q82419 . I'm going to merge this PR! |
Explanation
Added following steps for plugin guide -
Developing WasmEdge Plugins
Related issue- #85
Fixes #119
What type of PR is this
/kind documentation
Proposed Changes