feat: implement version sub command #911
Conversation
jglogan
left a comment
jglogan
left a comment
There was a problem hiding this comment.
Hi @fatelei, please have a look at the comments and lmk what you think!
Also for this to be merged you'll need so set up commit signing; see https://github.com/apple/containerization/blob/main/CONTRIBUTING.md#pull-requests for details.
| VolumeCommand.self | ||
| ] | ||
| ), | ||
| CommandGroup( |
There was a problem hiding this comment.
For the container CLI we've tried to avoid putting commands not related to container resources directly under container. Could you remove this command group, and make it so that the version command is a subcommand of container system?
| } | ||
|
|
||
| private func printVersionTable(_ info: VersionInfo) { | ||
| print("\(info.appName) version \(info.version)") |
There was a problem hiding this comment.
This isn't really tabular format. If we're going to have table and json format (which seems fine to me), the table format should use the TableOutput type that's used in all the resource.list() functions.
The other thing you could do for the table is try doing a ping() call on the API server and that would give you the API server release version if it's up (it should be the same but could differ if for example you did and upgrade and forgot to restart the API server, or had two installs and ran the wrong CLI executable). This could be an option (if the server responds) second row in the table.
If you do that, you could also add the buildType and appName values to what ping() returns for the API server.
| } | ||
| } | ||
|
|
||
| private func buildType() -> String { |
There was a problem hiding this comment.
How about relocating this into a public func in ReleaseVersion.swift?
You could then tweak ReleaseVersion.singleLine() to use ReleaseVersion.buildType() as well.
|
|
||
| public struct VersionInfo: Codable { | ||
| let version: String | ||
| let build: String |
There was a problem hiding this comment.
Make this buildType so the meaning is a little more obvious.
| OPTIONS: | ||
| --debug Enable debug output [environment: CONTAINER_DEBUG] | ||
| --version Show the version. | ||
| version Show detailed version information. |
2 participants
Type of Change
Motivation and Context
implement version sub command, give more info
Testing