-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Add support for different output formats on project view and project variables view command #328
base: main
Are you sure you want to change the base?
Conversation
Linked with #318 which looks to fix |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, I tested it locally and all is working well. Just one question but nothing blocking.
Also you might need to squash and merge this PR and include a conventional commit on the merge commit so we get a release note generated for this change
case constants.OutputFormatBasic, constants.OutputFormatTable: | ||
fmt.Fprintln(out, output.Bold(filteredVars[0].Name)) | ||
for _, v := range filteredVars { | ||
data := []*output.DataRow{} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this also print out the KEY VALUE
heading like the project view does?
data = append(data, output.NewDataRow(output.Bold("KEY"), output.Bold("VALUE"))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not against that change - although it perhaps should only apply to the table format (as it is with project view
) ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah definitely, it would probably be a bit confusing in the basic format
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've made a change in 98078bf but wanted to mention that Ben P mentioned he was going to review the view
command output format holistically. See this thread (internal link) for more details.
Added support for basic, json and table formats for the following octopus commands:
project view
project variable view
Partially fixes #312 (for
project view
Before
project view
:After
project view
:BASIC format
TABLE format:
JSON format:
Note: this PR does change the output to be in table view if no format is specified for
project view
. This seems logical given the default output format is table anyway.Before
project variable view
:After
project variable view
:BASIC and TABLE format: No change
JSON format: