Skip to content
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

fix some docs #239

Merged
merged 3 commits into from
Jul 10, 2024
Merged

fix some docs #239

merged 3 commits into from
Jul 10, 2024

Conversation

PeterD1524
Copy link
Contributor

I fixed some docs:

  • docs/contribute/source/plugin/wasi_nn.md
    • fix misplaced Appendix part for ggml backend
      • I think the cause is a2dd6ee put the Build WasmEdge with WASI-NN Neural Speed Backend part in the wrong place.
    • remove duplicate Apple Silicon Model part for ggml backend
      • The original has two identical Apple Silicon Model parts. One is in the MacOS section after Intel Model part and the other is in the Linux section after General Linux without any acceleration framework part.
        I removed the one in the Linux section after General Linux without any acceleration framework part.
      • I think this duplicate was removed in 26c0689 and wrongly added again in a2dd6ee.
  • docs/start/install.md
    • remove extra backtick

Copy link
Collaborator

alabulei1 commented Jul 4, 2024

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


Overall Summary:

Potential Issues and Errors:

  1. Duplication of "Appendix" section: The patch has inadvertently duplicated the "Appendix" section while moving it within the wasi_nn.md file. This redundancy should be addressed to avoid confusion and maintain document coherence.
  2. Incomplete Documentation due to section removal: Erasing the "Apple Silicon Model" section without explanation may cause ambiguity for users relying on that information, impacting their ability to configure WasmEdge on macOS arm64 platform correctly.

Most Important Findings:

  1. Correction of a minor typo: The patch also addresses a redundant backtick in the documentation text, enhancing readability.
  2. Useful Addition to the Appendix: The Appendix now includes valuable information on setting environment variables and lists pre-built ggml plugins on various platforms, which can benefit users.

Considering the need to address the duplication issue of the "Appendix" section and the necessity to ensure the integrity of the documentation by providing context for section removal, the patch requires further refinement before merging into the codebase. Additionally, the correction of the typo and the valuable additions to the Appendix enhance the overall documentation quality, indicating positive progress towards improving user experience and clarity.

Details

Commit bab4105221efad01228c709aa635f6fd9d40ddb3

Key Changes:

  • The patch fixes a misplaced "Appendix" section in the wasi_nn.md file under the docs/contribute/source/plugin/ directory.
  • It moves the Appendix section below the build instructions for the WasmEdge with WASI-NN Neural Speed Backend.
  • The Appendix now includes information on setting the WASMEDGE_PLUGIN_PATH environment variable and lists pre-built ggml plugins on different platforms.

Potential Problems:

  • The patch seems to have duplicated the "Appendix" section. It was originally present after the build instructions but was mistakenly moved up to the top of the file. This duplication should be resolved, and only one "Appendix" section should be retained in the correct position.
  • The information in the Appendix is useful, but it should only appear once in the document to avoid redundancy and confusion.
  • The patch should also mention why the Appendix section is located at the end and not in between the build instructions to maintain the logical flow of the document.

Commit cdee08cb5085e763083f408af44e92839e65b3a7

Key Changes:

  • The patch deletes a section titled "Apple Silicon Model" from the wasi_nn.md documentation file.

Potential Problems:

  • Incomplete Documentation: Removing the "Apple Silicon Model" section without explanation or context may lead to confusion for users who were relying on that information.
  • Loss of Information: If the section was relevant for users building and installing WasmEdge on macOS arm64 platform, its removal could impact their ability to correctly configure the build settings.

Recommendation:

  • Before removing sections from documentation, ensure that they are no longer relevant or have been adequately replaced.
  • Consider adding a brief note explaining the reason for removing the "Apple Silicon Model" section to provide transparency to users.

Commit c4aea2e2593341740821c6cde9ed3b6e3c2b38ca

Key Changes:

  • The patch fixes a typo in the documentation by correcting a redundant backtick that was mistakenly placed in the text.

Potential Problems:

  • The changes made in this patch are straightforward and aimed at fixing a minor typo in the documentation. There are no evident issues with the changes themselves.
  • It's always a good practice for the developer to ensure that the corrected text aligns with the project's writing style and guidelines to maintain consistency in the documentation.

Considering the simplicity and correctness of the changes, this patch seems ready for merging into the codebase.

Copy link
Member

@hydai hydai left a comment

Choose a reason for hiding this comment

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

LGTM

@PeterD1524 PeterD1524 mentioned this pull request Jul 4, 2024
@hydai hydai merged commit 872e691 into WasmEdge:main Jul 10, 2024
6 checks passed
@PeterD1524 PeterD1524 deleted the 1bbe9192a60f53cc branch July 11, 2024 06:00
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