Hi Martin, Thanks for the feedback. Yes "flagOptions" would be ok, may be we can add a description in the method comment to describe its purpose.
Thanks On Thu, Mar 12, 2015 at 6:58 AM, Martin Eppel (meppel) <mep...@cisco.com> wrote: > 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> > 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] > *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> > 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 > *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 > -- Imesh Gunaratne Technical Lead, WSO2 Committer & PMC Member, Apache Stratos
