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

Reply via email to