second-state / WasmEdge-WASINN-examples

Apache License 2.0
217 stars 35 forks source link

[CI] llama: add tests for plugins from registry #118

Closed dm4 closed 1 month ago

juntao commented 2 months ago

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


Commit 3da31d0aac3177ec74307809a91dbec914d754c2

Key changes:

  1. Addition of a new matrix variable plugin with options wasi_nn-ggml and wasi_nn-ggml-b2334.
  2. Modification in the job name to include the selected plugin.
  3. Modification in the installation step to use the selected plugin from the matrix.

Potential problems:

  1. The new matrix variable plugin may cause dependency or version compatibility issues with the existing configurations, especially if the plugins are not tested thoroughly.
  2. The use of curl with bash in the installation step can be a security risk as it installs components directly from a URL without verification.
  3. The patch lacks information about tests for the added plugins, which is crucial for verifying their functionality and ensuring they don't introduce regressions.
  4. Hardcoding the plugin versions in the installation script could lead to maintenance issues and difficulty in updating to newer versions.
  5. There might be a need to update the environment variable setup to accommodate any changes related to the added plugins.