-
Notifications
You must be signed in to change notification settings - Fork 892
feat: switch organization context in coder organizations #12265
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
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
3106518
to
9a7aba6
Compare
d4d20af
to
1ac16cf
Compare
7142630
to
b8ba53e
Compare
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 wouldn't say any of my comments are blockers, so I'm approving this.
This is a set of nice additions to the CLI!
if len(inv.Args) == 0 { | ||
// Pull switchToOrg from a prompt selector, rather than command line | ||
// args. | ||
switchToOrg, err = promptUserSelectOrg(inv, conf, orgs) |
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.
It'd be nice if the currently selected one is highlighted in the list, perhaps? 🤔
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.
It is! But I read it only from the config file. I ignore the CurrentOrganization
because that takes into account the -z
flag and is_default
.
If a user has nothing set, I want the reset option to be selected.
return org.Name == switchToOrg || org.ID.String() == switchToOrg | ||
}) | ||
if index < 0 { | ||
// Using this error for better error message formatting |
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.
We should really have one for CLI errors too. (PS. There is exitError
, but you might still prefer this approach.)
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.
Yea we really should. I did not want to make a new one for this.
exitError is a single line, this error format will display the helper text on a new line.
} | ||
|
||
// Verify it worked. | ||
current, err := CurrentOrganization(r, inv, client) |
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.
Do we need to do this, considering we've fetched orgs and are writing the UUID, what could go wrong?
I mostly worry about increased execution time for the CLI command by additional API roundtrip.
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 mainly added it because if you delete your selection, then this finds what the default is.
cli/organization.go
Outdated
defaultOrg = orgs[index].Name | ||
} | ||
|
||
const deselectOption = "--Deselect--" |
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 could also be [Default]
or the default could be highlighted (kylecarbs (Default)
). The latter would have a semantic difference, though, as it would never deselect. Not sure we need to teach users the concept of deselecting an org vs using the default which is essentially what we're doing 🤔
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 like [Default]
better than --Deselect--
. It is an annoying concept to include, but it felt wrong to not add the option of "Idc, let it choose for me".
My fear was someone runs this command to see what it does, then they are locked into a decision and unable to revert.
6b3367a
to
98666fd
Compare
1ac5298
to
b34cdfe
Compare
Closes #11927:
Adds a
coder org set
to switch the org context.coder org set