Merge lp:~jtv/gwacl/storage-tool-command-line into lp:gwacl
Status: | Merged |
---|---|
Approved by: | Jeroen T. Vermeulen |
Approved revision: | 223 |
Merged at revision: | 219 |
Proposed branch: | lp:~jtv/gwacl/storage-tool-command-line |
Merge into: | lp:gwacl |
Diff against target: |
386 lines (+203/-130) 1 file modified
example/storage/run.go (+203/-130) |
To merge this branch: | bzr merge lp:~jtv/gwacl/storage-tool-command-line |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Julian Edwards (community) | Approve | ||
Review via email: mp+180765@code.launchpad.net |
Commit message
Clean up command-line parsing in storage tool.
Description of the change
Actually I just wanted to add a "storage" option in order to fix bug 1213059: StorageContext.
(Go being Go, of course there's still some distraction in the code from complex, sophisticated procedures such as "now give me just the names").
And while working on that, I got irritated by the hopeless error output: you'd get one line of error, and then it would be completely drowned out by the usage output. And then, even if you were just asking for the usage information, you'd get a message about exit status. (Also, Go's flags package doesn't add the conventional "Usage:" to the usage output). So I added a -h option (Go's flags package doesn't seem to welcome GNU-style long and short forms for the same option, so no --help).
Now, invoking the program with -h prints usage and exits successfully. Getting the options wrong prints just a one-line pointer to the -h option, the actual error, and the exit status.
By the way, the Usage function was written as a variable instead of a regular function definition — probably because we were all new to Go at the time, but possibly also because the idea was to invoke the function through the flags package. In practice I find that the flags package adds no value at all (it doesn't add that "Usage:"!) so I simplified that a bit.
In some languages, you might wonder if "operation" shouldn't be a base class with different implementation types, or in Go, whether it should be an interface with different implementing structs. But that just adds massive overhead, for no particularly good reason. Yes, each has its own implementations of one or two "methods" but in this case I think it's perfectly valid to see that as an attribute rather than a type difference.
Branch with the actual fix for bug 1213059 follows.
Jeroen
Thanks for doing this, it was a bit messy in there. I've not given it massive scrutiny, since it's just an example program. If it runs and works that's fine by me.
You have two init() functions. I don't know if that's allowed or not but either way it's darn confusing, please collapse into a single func!