-
Notifications
You must be signed in to change notification settings - Fork 4
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
Team action improvements. #94
base: main
Are you sure you want to change the base?
Conversation
Here we're updating the team action to make a few improvements. * Allow multiple output formats for `list`, `info`, `update`. * Refactor positional argument for ID to flag `--team`. * Add examples to help output. * Change `enforce_sso` to `Bool` from `Bool?`. (API change) * Update/add new tests.
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.
Nice improvements.
My comments are mainly to add a default ouput method to avoid duplication.
def validate | ||
end |
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.
def validate | |
end | |
def validate | |
check_required_args do |missing| | |
missing << "team" unless team_id | |
end |
@@ -5,13 +5,53 @@ abstract class CB::TeamAction < CB::APIAction | |||
|
|||
format_setter format | |||
|
|||
private def output_team_details(t : CB::Client::Team) | |||
bool_setter show_header, true | |||
|
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 think this should be available as a teamAction method to avoid duplicate:
private def output_team(team: CB::Client::Team) | |
case @format | |
when Format::Default, Format::List | |
output_list team | |
when Format::JSON | |
output_json team | |
when Format::Table | |
output_table team | |
else | |
raise_invalid_format @format | |
end | |
end |
when Format::JSON | ||
output_json team | ||
else | ||
raise_invalid_format @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.
This will also allow list and table format
when Format::JSON | |
output_json team | |
else | |
raise_invalid_format @format | |
else | |
output(team) |
case @format | ||
when Format::Default, Format::List | ||
output_team_details(team) | ||
output_list team | ||
when Format::JSON | ||
output_json team | ||
when Format::Table | ||
output_json team | ||
else | ||
raise_invalid_format @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.
There is a typo in the when Format::Table
it should use output_table
But it could also be written:
output(team)
case @format | ||
when Format::Default, Format::List | ||
output_list team | ||
when Format::JSON | ||
output_json team | ||
when Format::Table | ||
output_table team | ||
else | ||
raise_invalid_format @format | ||
end |
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.
case @format | |
when Format::Default, Format::List | |
output_list team | |
when Format::JSON | |
output_json team | |
when Format::Table | |
output_table team | |
else | |
raise_invalid_format @format | |
end | |
output(team) |
Here we're updating the team action to make a few improvements.
list
,info
,update
.--team
.enforce_sso
toBool
fromBool?
. (API change)