Re: [PATCH datacenter-manager v3 4/6] cli: client: add `delete-token` option to delete token from remote

["Lukas Wagner" <l.wagner@proxmox.com>]

Hi,

some notes inline.

On Wed Feb 11, 2026 at 4:20 PM CET, Shan Shaji wrote:
> Signed-off-by: Shan Shaji <s.shaji@proxmox.com>
> ---
>  
>  **note:** No changes since v2
>  
>  cli/client/src/remotes.rs | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/cli/client/src/remotes.rs b/cli/client/src/remotes.rs
> index 4145dac..0b4df3c 100644
> --- a/cli/client/src/remotes.rs
> +++ b/cli/client/src/remotes.rs
> @@ -116,12 +116,17 @@ async fn update_remote(id: String, updater: RemoteUpdater) -> Result<(), Error>
>      input: {
>          properties: {
>              id: { schema: REMOTE_ID_SCHEMA },
> +            "delete-token": {
> +                type: bool,
> +                optional: true,
> +                description: "Remove the API-Token from remote."

Since this is the text shown in the help text in the CLI, it could be
nice to phrase more explicitly what exactly this flag is doing, for
instance:

  "Remove the API token used by Proxmox Datacenter Manager from the remote"


Also, same here as in the other patch, you can drop the `type` and use
`default: false` ...

> +            }
>          }
>      }
>  )]
> -/// Add a new remote.
> -async fn delete_remote(id: String) -> Result<(), Error> {
> -    client()?.delete_remote(&id).await?;
> +/// Delete a remote.
> +async fn delete_remote(id: String, delete_token: Option<bool>) -> Result<(), Error> {

... and then just have `delete_token: bool` here ...

> +    client()?.delete_remote(&id, &delete_token).await?;

... and Some(&delete_token) here.

>      Ok(())
>  }
>  

adding the default to the schema has the big benefit that these are
shown in the help text of the CLI tool:

	Optional parameters:

	 --delete-token <boolean>   (default=false)
		     Remove the API-Token from remote.