sercuzz8 commented on PR #8971:
URL: https://github.com/apache/storm/pull/8971#issuecomment-5226570773

   Hello @GGraziadei,
   thank you for the pieces of advice.
   1. I had overlooked the case, but I have now checked. `add_common_options` 
(`bin/storm.py:385-389`) declares `main_args` with `nargs='*'` and no 
`default=`, so it never holds  synthetic values from argparse. Only  `local()` 
(`bin/storm.py:1065`) prepend a non-CLI value in `main_args` and that runs 
*after* `raw_args.func(raw_args)` (and consequently the introduced method). No 
subcommand relies on a pre-merge injected default, so the replace semantics are 
safe;
   2. The proposed solution manages to be more elegant, concise and complete 
than the original one, implemented;
   3. It now falls back to the original functioning as this behaviour was 
accepted before;
   4. A new docstring with more information regarding input and output has been 
added. Consider accepting or rejecting it based on verbosity.


-- 
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