WasmEdge / wasmedge-rust-sdk

Embed WasmEdge functions in a Rust host app
Apache License 2.0
29 stars 15 forks source link

Update doc for releasing `v0.13.2` #93

Closed apepkuss closed 10 months ago

juntao commented 10 months ago

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


Overall Summary: This GitHub Pull Request titled "Update doc for releasing v0.13.2" includes multiple commits with various changes, mainly focusing on updating workflows, versioning tables, and documentation. While most of the changes seem to be straightforward and do not pose potential problems, there are a few issues that need attention.

The first potential problem lies in the paths-ignore configuration of the rust-static-lib workflow. It is not clear why these specific paths are added and how they may impact the desired behavior of the workflow.

The matrix configuration for the build_ubuntu job in the rust-static-lib workflow has been updated without stating the reasons for including specific Rust versions. Some context would be helpful to understand the motivation behind these changes.

Another notable issue is a typo in the cmake command in the run script, which includes the redundancy of "cmake cmake" instead of just "cmake."

Furthermore, the flag -DWASMEDGE_BUILD_TOOLS=OFF has been set in the cmake command, but it is unclear if this aligns with the intended changes, as it would exclude building the tools. Verification is needed to ensure the alignment.

Regarding the updates to the ci(rust-static-lib) workflow, the commented out Build WasmEdge with Release mode step requires clarification on whether it should be re-enabled or not. Additionally, the relationship between the Install dependencies step and the commented step needs clarification.

Overall, this pull request introduces various changes, with the most notable issues being the potential problems mentioned above. It is recommended that the author addresses these concerns and provides additional context where needed for better clarity and understanding.

Details

Commit 14614a0d44b9329af7cf7c6c275ffa01a195b6fd

Key changes:

Potential problems:

Commit 214c2e2d1f3f68bc98ff20c6b5890661b2153fbd

Key changes:

Potential problems:

Commit fb8da6d07a056b66af02fa9440bd043ad9358914

Key changes:

Potential problems:

Commit 3918ea63df7ef9bd58688f618fa72972f875cea0

Key changes:

Potential problems:

Commit 28086792e812707272ec9d34adc5b41e8f967241

Key changes:

Potential problems:

Commit 513f18e49f6b37ee695b0e17afd94abd23e0706c

Key changes:

Potential problems:

Commit 2adbc0af890a818b06a2d3b30081690660e5c468

Key changes:

Potential problems:

Commit 424706faef76f8863ab2f205048a913328ac7559

Key changes:

Potential problems:

Commit 0cfabfe292964ca834d8bf2f974e30da543d8ac1

Key changes:

Potential problems:

Overall, it seems that the main intention of this patch is to update the workflow by adding an Install dependencies step and an Install Rust-nightly step. Some clarification is needed regarding the commented out step and the ordering of the steps.

apepkuss commented 10 months ago

@hydai Could you please help review this PR? Please ignore the failed rust-static-lib workflow. We'll fix it in another PR. Thanks a lot!

apepkuss commented 10 months ago

@hydai Thanks for the review!