second-state / WasmEdge-WASINN-examples

Apache License 2.0
217 stars 35 forks source link

[CI] ggml: fix the execution error and enable macos runners #91

Closed hydai closed 3 months ago

juntao commented 3 months ago

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


Overall Summary: The pull request focuses on fixing an execution error and enabling the macOS runners for the ggml software. However, there are several potential issues and errors that need to be addressed. These include the lack of error handling, hard-coded paths and environment variables in the example command, the use of sudo within the CI workflow, and the absence of comments and explanations in the code. Additionally, there are duplicate steps in the workflow and potential unnecessary steps or commands. The submitter should provide more context for the changes and make the necessary improvements to address the potential problems.

Details

Commit a61eeccc0483f2879675d1a173a32e2c2f6f30ca

Key Changes:

Potential Problems:

Overall, the key changes seem to focus on fixing an execution error and enabling the macOS runners. However, the patch could be improved by addressing the potential problems mentioned above.

Commit 47756b003bbecbc789b103f224b906d72978bc31

Key changes:

Potential problems:

  1. The patch does not include any comments explaining the reasons for the changes and their impact.
  2. The patch introduces duplicate steps in the workflow. The steps for the "Tiny Llama" job and the "llama2 7b" job are identical.
  3. The usage of sudo command in the "Tiny Llama" job might cause permission issues depending on the workflow environment.
  4. The patch may include unnecessary steps or commands that could be optimized or removed.

It is recommended for the submitter to provide more context about the changes and address the potential problems mentioned above.