search

build-trust / ockam

Orchestrate end-to-end encryption, cryptographic identities, mutual authentication, and authorization policies between distributed applications – at massive scale.
https://ockam.io
Apache License 2.0
4.43k stars 560 forks source link

Replace `timeout` argument type on `ockam status` command to `Duration` #6374

Open adrianbenavides opened 11 months ago

adrianbenavides commented 11 months ago

Current behavior

The timeout argument on ockam status is of type u64.

Desired behavior

The timeout argument on ockam status should be of type Duration and its doc string should have removed the (in seconds) part. Also, the default value should be changed to 30s and the value_parser argument should be added calling the duration_parser function.

Take this as an example:

https://github.com/build-trust/ockam/blob/59f52a66cc84ad682d9f4ea5715057f97f8eb7a3/implementations/rust/ockam/ockam_command/src/message/send.rs#L47-L49

Implementation details

This is the argument that must be changed:

https://github.com/build-trust/ockam/blob/5019283dfcb3ed95853e3729c2313f0e475a788e/implementations/rust/ockam/ockam_command/src/status.rs#L31-L33

Some tips for newcomers:

  1. Create a new branch for the issue you are working on (don't push directly to your fork's develop branch)
  2. Run cargo fmt. You can configure the editor to run it automatically everytime a file is saved
  3. Run cargo clippy to make sure the code compiles + follows rust standards/best practices
  4. You don't need to keep your branch updated with develop until there is a merge conflict. If you still want to update it, use git rebase develop instead of gir merge develop

We love helping new contributors! ❤️ If you have questions or need help as you explore, please join us on Discord. If you're looking for other issues to contribute to, please checkout our good first issues.

tibormichel1 commented 11 months ago

Hello, i would be interested to work on this issue, but is it possible to add something more to do ? (it is requirement by university task about 3-4 hrs of work) thanks :)

adrianbenavides commented 11 months ago

All yours!

but is it possible to add something more to do ? (it is requirement by university task about 3-4 hrs of work)

I can't extend this issue artifically, but we have plenty of open issues. You can grab as many as you want 👍

tibormichel1 commented 11 months ago

Thanks, but it is neccessary to be single task :D is it possible to make task with subtasks https://github.com/build-trust/ockam/issues/6376 https://github.com/build-trust/ockam/issues/6375 https://github.com/build-trust/ockam/issues/6374 or somehow connect them here on github ?

adrianbenavides commented 11 months ago

You can reference multiple issues from a single PR, if that works for you.