You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by "koushiro (via GitHub)" <gi...@apache.org> on 2024/10/23 08:49:15 UTC
[PR] refactor(bin/oli): use `clap_derive` to reduce boilerplate code [opendal]
koushiro opened a new pull request, #5233:
URL: https://github.com/apache/opendal/pull/5233
# Which issue does this PR close?
Closes #.
# Rationale for this change
reduce boilerplate code
# What changes are included in this PR?
use `clap_derive` (instead of `clap_builder`) to reduce boilerplate code
# Are there any user-facing changes?
No
--
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] refactor(bin/oli): use `clap_derive` to reduce boilerplate code [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #5233:
URL: https://github.com/apache/opendal/pull/5233#issuecomment-2431576866
> I add a new type `ConfigArgs` (or `ConfigParams`?) in [3808562](https://github.com/apache/opendal/commit/3808562a1347a806919913e04b3cae5e33e55b3f) to reduce the same argument declaration, or you think it's not necessary?
Make sense to me.
--
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] refactor(bin/oli): use `clap_derive` to reduce boilerplate code [opendal]
Posted by "koushiro (via GitHub)" <gi...@apache.org>.
koushiro commented on PR #5233:
URL: https://github.com/apache/opendal/pull/5233#issuecomment-2431568173
I add a new arg type `ConfigArgs` (or `ConfigParams`?) in https://github.com/apache/opendal/pull/5233/commits/3808562a1347a806919913e04b3cae5e33e55b3f to reduce the same argument declaration, or you think it's not necessary?
--
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] refactor(bin/oli): use `clap_derive` to reduce boilerplate code [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #5233:
URL: https://github.com/apache/opendal/pull/5233#issuecomment-2431359650
Hi, I implemented this to support the use of a proxy as follows:
```
ln oli ocp
ocp <src> <dst>
ln oli ols
ols <src>
```
Users can use `ocp` in exactly the same way as `oli cp`.
After this change, can we still operate in this manner?
--
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] refactor(bin/oli): use `clap_derive` to reduce boilerplate code [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #5233:
URL: https://github.com/apache/opendal/pull/5233#issuecomment-2431568867
> 2\. If we use `clap_derive` and want to support the proxy manner that you mentioned, then we cannot use the global default config arg, and we need to add a default config for each cmd, can you accept this implementation?
Hi, that's look to me.
--
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] refactor(bin/oli): use `clap_derive` to reduce boilerplate code [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #5233:
URL: https://github.com/apache/opendal/pull/5233#issuecomment-2431593974
> BTW, do we need to add oli's tests to CI or just keep it.
Including an oli test in the CI would be appreciated.
--
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] refactor(bin/oli): use `clap_derive` to reduce boilerplate code [opendal]
Posted by "koushiro (via GitHub)" <gi...@apache.org>.
koushiro commented on PR #5233:
URL: https://github.com/apache/opendal/pull/5233#issuecomment-2431464574
> Hi, I implemented this to support the use of a proxy as follows:
>
> ```
> ln oli ocp
> ocp <src> <dst>
>
> ln oli ols
> ols <src>
> ```
>
> Users can use `ocp` in exactly the same way as `oli cp`.
>
I encountered two problems:
1. The previous (before this PR) implementation had the following problem:
```
➜ oli git:(main) ln target/release/oli ols
➜ oli git:(main) ✗ ./ols src
error: 'ols' requires a subcommand but one was not provided
Usage: ols [OPTIONS] <target>
For more information, try '--help'.
```
2. If we use `clap_derive` and want to support the proxy manner that you mentioned, then we cannot use the global default config arg, and we need to add a default config for each cmd, can you accept this implementation?
--
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] refactor(bin/oli): use `clap_derive` to reduce boilerplate code [opendal]
Posted by "koushiro (via GitHub)" <gi...@apache.org>.
koushiro commented on PR #5233:
URL: https://github.com/apache/opendal/pull/5233#issuecomment-2431588442
@Xuanwo All done, PTAL
BTW, do we need to add oli's tests to CI or just keep it.
--
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] refactor(bin/oli): use `clap_derive` to reduce boilerplate code [opendal]
Posted by "koushiro (via GitHub)" <gi...@apache.org>.
koushiro commented on PR #5233:
URL: https://github.com/apache/opendal/pull/5233#issuecomment-2431351095
oli's tests are not currently checked in CI
```
➜ oli git:(use-clap-derive) ✗ cargo test --release --all-features
Compiling oli v0.41.12 (/Users/qinxuan/Code/rust-lang/opendal/bin/oli)
Finished `release` profile [optimized] target(s) in 37.08s
Running unittests src/lib.rs (target/release/deps/oli-c04c0e204779fc76)
running 7 tests
test config::tests::test_load_from_env ... ok
test config::tests::test_parse_s3_location3 ... ok
test config::tests::test_parse_fs_location ... ok
test config::tests::test_load_from_toml ... ok
test config::tests::test_load_config_from_file_and_env ... ok
test config::tests::test_parse_s3_location ... ok
test config::tests::test_parse_s3_location2 ... ok
test result: ok. 7 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
Running unittests src/bin/oli.rs (target/release/deps/oli-139e3cb8d6e37d4c)
running 0 tests
test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
Running tests/cat.rs (target/release/deps/cat-7fa8be95eddce583)
running 2 tests
test test_cat_for_path_in_current_dir ... ok
test test_basic_cat ... ok
test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.51s
Running tests/cp.rs (target/release/deps/cp-f883dfcf27676c1a)
running 2 tests
test test_basic_cp ... ok
test test_cp_for_path_in_current_dir ... ok
test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.03s
Running tests/ls.rs (target/release/deps/ls-c1a38755bc9e6f86)
running 1 test
test test_basic_ls ... ok
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.01s
Running tests/rm.rs (target/release/deps/rm-071bb9fe37b27c4c)
running 2 tests
test test_basic_rm ... ok
test test_rm_for_path_in_current_dir ... ok
test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.01s
Running tests/stat.rs (target/release/deps/stat-ca4edaf8b3ac28e0)
running 2 tests
test test_stat_for_path_in_current_dir ... ok
test test_basic_stat ... ok
test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.01s
Doc-tests oli
running 0 tests
test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
➜ oli git:(use-clap-derive) ✗
```
--
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] refactor(bin/oli): use `clap_derive` to reduce boilerplate code [opendal]
Posted by "koushiro (via GitHub)" <gi...@apache.org>.
koushiro commented on PR #5233:
URL: https://github.com/apache/opendal/pull/5233#issuecomment-2431369034
> Hi, I implemented this to support the use of a proxy as follows:
>
> ```
> ln oli ocp
> ocp <src> <dst>
>
> ln oli ols
> ols <src>
> ```
>
> Users can use `ocp` in exactly the same way as `oli cp`.
>
> After this change, can we still operate in this manner?
No, but I can try to support this manner.
--
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] refactor(bin/oli): use `clap_derive` to reduce boilerplate code [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #5233:
URL: https://github.com/apache/opendal/pull/5233#issuecomment-2431385912
> Do you think the other parts are ok?
Other parts looks good to me, thank you for working on this!
--
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] refactor(bin/oli): use `clap_derive` to reduce boilerplate code [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo merged PR #5233:
URL: https://github.com/apache/opendal/pull/5233
--
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