upbound / up

The @upbound CLI
Apache License 2.0
52 stars 41 forks source link

Updated user and invite commands for org to be consistent with other commands #306

Closed AlainRoy closed 1 year ago

AlainRoy commented 1 year ago

Description

This PR reorganizes the user and invite commands to match the style of other commands.

For the user commands, we have changed the following:

up org user list-members
up org user remove-member

to:

up org user list
up org user remove

For the invite commands, we have changed the following:

up org user invite
up org user list-invites
up org user delete-invite

to:

up org invite create
up org invite list
up org invite delete

The implementation is unchanged--the only changes are to move around the commands.

I have:

How has this code been tested

Manual testing.

tnthornton commented 1 year ago

The PR looks pretty straightforward. I do have a question about the following:

For the invite commands, we have changed the following:

up org user invite up org user list-invites up org user delete-invite to:

up org invite create up org invite list up org invite delete

From a UX perspective, I'm trying to understand the change. Previously, we had: executable subject resource action

now it seems like we have: executable subject action method

Are there parts of the invocation that just aren't listed? With up org user invite it was clear that within an org, you're inviting a user. With up org invite create it's clear you're creating an invite, but for what?

AlainRoy commented 1 year ago

I think the revised style matches what we do in the ctp command:

up ctp create/get/list/delete
up ctp pull-secret create

And in the robot command:

up robot create/get/list/delete
up robot token create/delete/list

That is, we have commands of the form:

up <type> create/get/list/delete
up <type> <subtype> create/get/list/delete

I matched that style for users and invites.

One reason I like it is the consistent actions. We consistently have "list" instead of "list-members" and "list-invites", for example.

I think I made the command more consistent. I'm happy to chat about it tomorrow if you like.

tnthornton commented 1 year ago

I matched that style for users and invites.

One reason I like it is the consistent actions. We consistently have "list" instead of "list-members" and "list-invites", for example.

The intent makes sense. I think the part I'm getting hung up on is that with the other examples, we're acting on 1 type/resource at a time. Whereas with the org operations we have multiple things happening:

Invites to an org - listing at a top level makes complete sense. Invite a user to an org - there's an action and 2 resources. Delete a user invite to an org - there's an action and 2 resources.

Happy to chat more about this tomorrow 👍

AlainRoy commented 1 year ago

Ah, I see what you're saying.

While I wasn't fond of style of the original version, I'm happy to figure out something we both like. I'm happy to chat about it when you have time tomorrow.

AlainRoy commented 1 year ago

Taylor and I talked through this and came up with a better proposal. I'll close this PR and will soon open a new one with the new approach.