Skip to content
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

fix(cli): add organization back to bucket response #15947

Merged
merged 1 commit into from
Nov 18, 2019

Conversation

imogenkinsman
Copy link
Contributor

@imogenkinsman imogenkinsman commented Nov 16, 2019

ref #15828

This fixes a mismatch between tabwriter keys introduced by 9d44ac3.

It's probably worth considering adding org names back in the response. Expecting users to work with orgIDs for everything feels like bad UX when it's really an internal implementation detail.

@imogenkinsman imogenkinsman requested review from a team November 18, 2019 17:25
@ghost ghost requested review from kelwang and removed request for a team November 18, 2019 17:25
Copy link
Contributor

@jsteenb2 jsteenb2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link
Contributor

@kelwang kelwang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't looks like it's gonna help anything. @jademcgough could you explain a little more about this?

@imogenkinsman
Copy link
Contributor Author

imogenkinsman commented Nov 18, 2019

@kelwang organization IDs do not show for buckets in the CLI currently because the keys specified in the tab writer body don't match those in the tab writer header. This fixes that.

Specifically, one of them is "OrgID" and the other is "OrganizationID".

@kelwang
Copy link
Contributor

kelwang commented Nov 18, 2019

Ah, okay. Could you change OrganizationID to OrgID, seems most of our code base is using OrgID instead.

@imogenkinsman
Copy link
Contributor Author

@kelwang we use organizationID in all other places in our CLI, so this is consistent with that. I'd rather merge this and have that be a separate issue if we want it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants