Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

fleetctl: Need return fail if no args given to some subcommand. #1230

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

wuqixuan
Copy link
Contributor

Some subcommand like destroy/load/start/status/stop/submit/unload,
need to give prompt if no args for them.
Add the same prompt "One unit file must be provided" as the "cat"
subcommand is doing:
[root@localhost fleet-0.10.1]# fleetctl cat
One unit file must be provided

Some subcommand like destroy/load/start/status/stop/submit/unload,
need to give prompt if no args for them.
Add the same prompt "One unit file must be provided" as the "cat"
subcommand is doing:
[root@localhost fleet-0.10.1]# fleetctl cat
One unit file must be provided
@wuqixuan
Copy link
Contributor Author

Before the code, destroy/load/start/status/stop/submit/unload subcommand without any args will give 0 return value. It should return error code, like -1 as "cat" is doing now. That's why to add those codes.

@scole-scea
Copy link

Maybe: "At least one unit file must be specified."

@wuqixuan
Copy link
Contributor Author

Currently, "cat" subcommand give "One unit file must be provided". So other subcommand I gave the same.

@wuqixuan wuqixuan changed the title fleetctl: Add the args check for some subcommand fleetctl: Need return fail if no correct args for some subcommand. May 19, 2015
@wuqixuan wuqixuan changed the title fleetctl: Need return fail if no correct args for some subcommand. fleetctl: Need return fail if no args given to some subcommand. May 19, 2015
@scole-scea
Copy link

Yes, the cat subcommand does say "one". However, the cat subcommand only takes one unit as an argument.

destroy/load/start/status/stop/submit/unload all take one or more units as arguments.

@wuqixuan
Copy link
Contributor Author

So I think we need modify the message of "cat", instead.

@scole-scea
Copy link

Well, cat has a different command-line interface. There's no reason it's error messages shouldn't reflect that difference.

@@ -29,6 +29,10 @@ Destroyed units are impossible to start unless re-submitted.`,
}

func runDestroyUnits(args []string) (exit int) {
if len(args) == 0 {
stderr("One unit file must be provided.")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One or more unit files must be provided

@wuqixuan
Copy link
Contributor Author

@bcwaldon, are all commands or just stop need not give error if no args failing ?
If all commands, then "cat" code need remove the check?

func runCatUnit(args []string) (exit int) {
    if len(args) != 1 {
        stderr("One unit file must be provided")
        return 1
    }

Or some need check, some need not check ? 

@mischief
Copy link
Contributor

@wuqixuan please update the messages as @bcwaldon asked and then lgtm.

@wuqixuan
Copy link
Contributor Author

wuqixuan commented Sep 1, 2015

@mischief, actually I don't know what @bcwaldon exact suggestions are.
Which commands are idempotent without checking, while which need to be checked ?

@dongsupark dongsupark force-pushed the master branch 2 times, most recently from 14580b0 to 63b20dc Compare June 22, 2016 10:22
@dongsupark dongsupark force-pushed the master branch 3 times, most recently from 23d4b13 to 6fb1256 Compare July 1, 2016 11:07
@dongsupark dongsupark force-pushed the master branch 2 times, most recently from 54409ab to a1a21b8 Compare July 8, 2016 11:38
@dongsupark dongsupark force-pushed the master branch 3 times, most recently from 150d30c to 4002bf5 Compare August 16, 2016 08:54
@dongsupark dongsupark force-pushed the master branch 7 times, most recently from 3b60e93 to 875d938 Compare August 23, 2016 11:00
@dongsupark dongsupark force-pushed the master branch 2 times, most recently from bdc94e8 to fa5aa3a Compare August 30, 2016 12:26
@dongsupark dongsupark force-pushed the master branch 3 times, most recently from 20a3e96 to 3aaa1ab Compare November 10, 2016 15:24
@dongsupark dongsupark force-pushed the master branch 2 times, most recently from eb6872f to 365565e Compare November 24, 2016 15:35
@dongsupark dongsupark force-pushed the master branch 6 times, most recently from 39a99ba to 44591b0 Compare December 15, 2016 19:48
@dongsupark dongsupark force-pushed the master branch 3 times, most recently from 0132632 to 6974811 Compare February 8, 2017 10:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants