second-state / wasmedge-quickjs

A high-performance, secure, extensible, and OCI-complaint JavaScript runtime for WasmEdge.
Apache License 2.0
494 stars 60 forks source link

Rename httpx::Buffer::length into httpx::Buffer::byteLength #103

Closed Wonshtrum closed 8 months ago

Wonshtrum commented 1 year ago

Fix for #102.

juntao commented 1 year ago

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


Commit ebac18ab5887b0abc68d67c41b87242977e8343c

The pull request changes the name of a method httpx::Buffer::length to httpx::Buffer::byteLength in js_module.rs file.

The proposed change looks appropriate as this correctly reflects that the length property for Buffer actually returns the length of the buffer in bytes, not the number of characters in the buffer. The pull request also references the issue that this change fixes.

There should not be any potential problems with this change as it looks to be a minor change that only affects the name of a method in a single file.

juntao commented 1 year ago

Thanks! @L-jasmine please review.

L-jasmine commented 1 year ago

Thank you for your pull request, but I'm concerned about potential compatibility issues. Therefore, I would prefer to add a new 'byteLength' instead of modifying the existing one.

L-jasmine commented 8 months ago

Thank you for your pull request. Similar changes have already been incorporated in the last update, so I won't merge this pull request. Nevertheless, I appreciate your contribution.