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

[doc] update docs #28

Merged
merged 5 commits into from
Jul 21, 2023
Merged

[doc] update docs #28

merged 5 commits into from
Jul 21, 2023

Conversation

apepkuss
Copy link
Collaborator

In this PR, update READMEs and rustdocs.

Copy link
Member

juntao commented Jul 21, 2023

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


Overall, the pull request titled "[doc] update docs" includes several individual patches. Here are the most important findings and potential issues to address:

  1. The removal of the async feature in the "Build API document" job lacks clear explanation in the commit message or PR description. This lack of context may confuse future developers and it would be beneficial to provide a rationale for this change.

  2. The patch that updates references to error types in documentation comments seems to be correct and straightforward without any identified issues.

  3. The patch that updates the rustdoc, removes unnecessary comments, and updates documentation for the add_host_function method does not include any code changes. It would be advisable to confirm with the author if this is intentional or if there are missing code changes in the patch. Additionally, it would be helpful to have explanations for the removal of comments to aid in understanding.

  4. The patch that updates multiple files, including the README.md, src/instance/function.rs, src/instance/module.rs, and src/lib.rs, addresses various changes related to version information and documentation comments. Some potential problems to address are verifying the accuracy and compatibility of the updated version information, reviewing documentation comments for clarity and consistency, and adding more detailed information about the changes in commit messages or the PR description.

  5. The update to the version number in the Cargo.toml file and the addition of new dependency versions in the README.md file seem straightforward and do not introduce any potential problems.

In summary, while most of the patches seem correct, some potential issues and areas for improvement have been identified. To ensure clarity and maintain the functionality of the codebase, it is recommended to provide more explanation for changes, review documentation comments, and confirm if any missing code changes are intentional.

Details

Commit 780b367cfa90f7f7b5fca1b894b578b06a19eaef

Key changes:

  • The cargo doc command in the "Build API document" job has been modified to remove the async feature.

Potential problems:

  • It seems that the async feature has been removed deliberately, but there is no explanation provided in the commit message or PR description. It would be helpful to have a clear reason for this change.
  • The comment in the code indicates that the async feature was previously included, but it is not clear why it is being removed now. This may cause confusion for future developers.
  • It would be advantageous to have additional context on the reasoning behind these changes and the impact they may have on the overall functionality of the codebase.

Commit 68e51aa9edf7443c2010fbaf5fad07a7000a022a

Key Changes:

  • Updated references to error types in documentation comments to use the wasmedge_types::error module instead of the crate::error module.

Potential Problems:

  • No potential problems were identified in this patch.

Overall, the patch seems to be straightforward and correct. It updates the documentation comments in several files to reference the correct error types.

Commit 6f8f2b6df462bcefb1e7539a53e4f07480954e8a

Key changes:

  • The patch updates the rustdoc in the io.rs file.
  • It removes unnecessary comments regarding the run_async methods in multiple places.
  • It updates the documentation for the add_host_function method in the plugin.rs file.

Potential problems:

  • The patch removes some comments without any explanation. It would be helpful to add a comment explaining why the comments were removed.
  • The patch does not include any code changes, only documentation updates. It would be good to confirm with the author if this is intentional or if there are missing code changes in the patch.

Commit 3e6c54c45aa05d94c88f74bdcf2ec084d2baf382

Key changes in this patch include:

  1. The README.md file in the crates/wasmedge-sys directory has been updated to include a table of version information for the dependencies.
  2. The src/instance/function.rs file has been modified to update the documentation comments for the data parameter in several function definitions.
  3. The src/instance/module.rs file has been modified to update the documentation comments for the host_data parameter in the ImportModule struct.
  4. The src/lib.rs file has been modified to update the version information table in the documentation comments.

Key potential problems to address in this patch are:

  1. The updated version information in the README.md and lib.rs files should be verified for accuracy and compatibility with the project dependencies.
  2. The documentation comments in the src/instance/function.rs and src/instance/module.rs files should be reviewed for clarity and consistency.
  3. It would be helpful to include more detailed information about the changes being made in the commit message or pull request description.

Commit 4fbf44636a5a987c9eb6e15eb37027ffbcb3286f

Key Changes:

  • The version in the Cargo.toml file has been updated from "0.10.0-dev" to "0.10.0".
  • The README.md file has been updated to include new versions of dependencies.

Potential Problems:

  • There are no potential problems identified in this patch.

Overall, this patch updates the version number in the Cargo.toml file and adds new versions of dependencies to the README.md file. The changes seem straightforward and do not introduce any potential problems.

@apepkuss apepkuss requested a review from L-jasmine July 21, 2023 09:24
@apepkuss apepkuss merged commit c24fd4c into WasmEdge:main Jul 21, 2023
@apepkuss
Copy link
Collaborator Author

@L-jasmine Thanks for the review!

@apepkuss apepkuss deleted the doc/update-docs branch July 21, 2023 10:05
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