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 java code examples in README #3092

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

MichaelMorrisEst
Copy link

Fixes #3091

Signed-off-by: MichaelMorris <michael.morris@est.tech>
Copy link
Member

juntao commented Dec 14, 2023

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


Commit 6cd6f2b11735f87609f684da9aa019bf4754175a

This patch contains a significant refactoring of the Java code examples in the README file for a project related to running WebAssembly (Wasm) from Java.

Summary of changes:

  • Introduction of vars fibWasmPath and funcName intended to improve readability and reduce hardcoding.
  • Change in the use of loading file content into a byte array from an unshown loadFile method to Files.readAllBytes().
  • Class WasmEdgeValue is replaced with Value.
  • WasmEdgeAsync is replaced with Async.
  • Several functions appear to be renamed to align more with Java naming conventions. For example: wasmEdge_AsyncWait becomes asyncWait, wasmEdge_AsyncGet() becomes get(), etc.
  • Instructions on managing execution results and canceling operations are updated with the new function names.

Potential issues:

  1. No corresponding changes in the actual Java code are provided in this patch. If these are only changes in the README, they might not reflect the codebase, leading to confusion.
  2. If classes like WasmEdgeValue and WasmEdgeAsync are renamed, and this change is not limited to the README but reflected in the whole project, it may be a breaking change for projects that depend on this software.
  3. New dependencies on java.nio.file.Files and java.nio.file.Paths classes are introduced. They should be checked for their compatibility in the project's target environments.

This pull request does lack a context to understand completely without the actual source code. Reviewing the actual code changes implementing these refactorings would be beneficial.

@github-actions github-actions bot added the binding-java Java Bindings label Dec 14, 2023
@dannypsnl
Copy link
Member

dannypsnl commented Dec 15, 2023

It seems before this you need to fix the binding, from my check, the CI failed in a week.

@alabulei1
Copy link
Contributor

Hi @MichaelMorrisEst ,

Can you please check out the CI tests? Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
binding-java Java Bindings
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: Outdated README in java bindings
4 participants