Hi Imesh, I don’t disagree, I do agree – no problem with changing the name of the variable but “options” simply doesn’t work (it won’t compile), we need to find another, non-clashing name.
What about “flagOptions” ? Regards Martin From: Imesh Gunaratne [mailto:im...@apache.org] Sent: Wednesday, March 11, 2015 6:13 PM To: dev Subject: Re: DISCUSS: usage of flags in stratos cli 4.1 Hi Martin, Thanks for the response. Still "cmdOptions" does not sound (at least to me) as they are passed by the shell when running the CLI in none interactive mode. For a new developer it would be difficult to understand why this mergeOptions() method is executed and why we are reading command options twice. If we put proper variable names it would be much easier to understand. WDYT? Thanks On Wed, Mar 11, 2015 at 2:19 AM, Martin Eppel (meppel) <mep...@cisco.com<mailto:mep...@cisco.com>> wrote: In general sounds good but I think naming it “options” won’t work as the cmd classes already have a field named “options” and they will clash, what about “cmdOptions” ? Thanks Martin e.g.: … public class AddAutoscalingPolicyCommand implements Command<StratosCommandContext> { private static final Logger logger = LoggerFactory.getLogger(AddAutoscalingPolicyCommand.class); private final Options options; … From: Imesh Gunaratne [mailto:im...@apache.org<mailto:im...@apache.org>] Sent: Tuesday, March 10, 2015 1:08 PM To: dev Subject: Re: DISCUSS: usage of flags in stratos cli 4.1 Hi Martin, I reviewed your fix and it looks good. The change you have done to include the options passed by the command line when running the CLI straightaway from the shell is correct. I would like to propose one change: you have named the new argument as "already_parsed_opts": int execute(T context, String[] args, Option[] already_parsed_opts) throws CommandException; May be we need to use standard Java naming convention here. Moreover "alreadyParsedOpts" does not give me a proper meaning, shall we rename it to "options"? Then anyone who would implement a command would know that the execute method itself passes options and we can also use commandLine.getOptions() to read options passed when running in interactive mode. Thanks On Tue, Mar 10, 2015 at 10:52 AM, Martin Eppel (meppel) <mep...@cisco.com<mailto:mep...@cisco.com>> wrote: I pushed the bug fix in commit 06deaadc63a9756e7701f5173ba00847aec24c4a – please review. Although it seems that currently we are not using any commands with the flag only option it might still prove to be useful in case it will be needed. If you see an issue or concern please feel free to revert, Thanks Martin From: Martin Eppel (meppel) Sent: Monday, March 09, 2015 12:01 PM To: dev@stratos.apache.org<mailto:dev@stratos.apache.org> Subject: DISCUSS: usage of flags in stratos cli 4.1 Hi, In 4.1 do we still support cli commands which accept flags only, similar to “unsubscribe-cartridge –f” in 4.0 ? I went through all the cli commands but don’t see any “flag” as command option (unless I missed it) . The reason I am asking is that in 4.0.0 there was an issue with the command parser which missed to properly parse flags when there were invoked in the command mode (although it worked in the interactive cli mode). I have a fix for this issue (ported from 4.0.0 private branch), however, if we don’t support flags then there is no need to push it upstream unless we plan to do so in the future? Thanks Martin -- Imesh Gunaratne Technical Lead, WSO2 Committer & PMC Member, Apache Stratos -- Imesh Gunaratne Technical Lead, WSO2 Committer & PMC Member, Apache Stratos
