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
