You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by "yihong0618 (via GitHub)" <gi...@apache.org> on 2025/03/09 10:09:22 UTC
[PR] ci: add clippy to some of the binding check ci [opendal]
yihong0618 opened a new pull request, #5717:
URL: https://github.com/apache/opendal/pull/5717
# Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and enhancements. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
-->
Follow the comment in https://github.com/apache/opendal/pull/5708#pullrequestreview-2669216589
this patch add clippy to some of the bindings
and make the clippy happy of them
Also found some api had changed for them, this patch does not change them instead of add `FIXME` comment
for later discuss or fix
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ci: add clippy to some of the binding check ci [opendal]
Posted by "oowl (via GitHub)" <gi...@apache.org>.
oowl commented on code in PR #5717:
URL: https://github.com/apache/opendal/pull/5717#discussion_r1986310919
##########
bindings/nodejs/src/lib.rs:
##########
@@ -131,7 +131,8 @@ impl Operator {
/// ```
#[napi]
pub async fn is_exist(&self, path: String) -> Result<bool> {
- self.0.is_exist(&path).await.map_err(format_napi_error)
+ // FIXME(yihong0618) use the same api exists
Review Comment:
let's remove this line
##########
bindings/lua/src/lib.rs:
##########
@@ -68,7 +68,7 @@ fn operator_new<'a>(
operator.set("read", lua.create_function(operator_read)?)?;
operator.set("write", lua.create_function(operator_write)?)?;
operator.set("delete", lua.create_function(operator_delete)?)?;
- operator.set("is_exist", lua.create_function(operator_is_exist)?)?;
+ operator.set("is_exist", lua.create_function(operator_is_exist)?)?; // FIXME(yihong0618) use the same api exists
Review Comment:
```suggestion
operator.set("is_exist", lua.create_function(operator_is_exist)?)?;
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ci: add clippy to some of the binding check ci [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo merged PR #5717:
URL: https://github.com/apache/opendal/pull/5717
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ci: add clippy to some of the binding check ci [opendal]
Posted by "yihong0618 (via GitHub)" <gi...@apache.org>.
yihong0618 commented on code in PR #5717:
URL: https://github.com/apache/opendal/pull/5717#discussion_r1986312593
##########
bindings/nodejs/src/lib.rs:
##########
@@ -131,7 +131,8 @@ impl Operator {
/// ```
#[napi]
pub async fn is_exist(&self, path: String) -> Result<bool> {
- self.0.is_exist(&path).await.map_err(format_napi_error)
+ // FIXME(yihong0618) use the same api exists
Review Comment:
copy
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org