tengqm commented on PR #6198:
URL: https://github.com/apache/gravitino/pull/6198#issuecomment-2590120048

   > It was added at the request of @tengqm with the advice of https://clig.dev.
   > 
   > For example: "For instances where you do want no output (for example, when 
used in shell scripts), to avoid clumsy redirection of stderr to /dev/null, you 
can provide a -q option to suppress all non-essential output."
   
   Right. This change seems like a compromise between stricter traditional 
command outputs and something advised elsewhere. In some reviews to CLI command 
outputs (cannot recall exactly which one thought), I raised my concern about 
the necessity of printing success messages, such as "something has been 
successfully removed/updated". I was more inclined to _explicit, ationable 
error messages_ when things go bad, or _silent success with no output_ if an 
acction succeeded. We are not creating a chatbot with the CLI tool after all. 
Verbose outputs may also complicate the scriptability of the CLI command. There 
were other arguments that this verbose messages are actually desirable, and 
they can be suppressed using a `-q` argument. I tend to agree with this 
approach. That is the background.
   
   Back to the implementation, I am a little bit sick of the actual 
implementation at this moment. Without an appropriate schema to group or to 
abstract the command line arguments, this kind of changes won't be uncommon in 
the foreseeable future. For example, for all `list` operations, we may want a 
`--sort` option, or even a `--sort-order` option; for another example, we may 
want filtering, paging options.
   
   A cleaner solution, on top of my head, is to have something like this:
   
   - a `GenericOptions` for flags like `url`, `metalake`, `authenticate`, 
`ignore-version`, `verbose` etc.
   - a `ListOptions` for flags like `sort`, `output-format`, `filter`, ...
   - a `DeleteOptions` for flags like `force`, `cascaded`, `forever`, etc.
   
   We are not there yet. But if we are on the same page, knowing where we are 
heading, this change should be fine. 
   


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to