-
Notifications
You must be signed in to change notification settings - Fork 881
chore: implement cli list organization members #13555
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
Emyrk
commented
Jun 12, 2024
•
edited
Loading
edited
b64c69b
to
dfded68
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.
all stacks look good so far and make sense to me
OrganizationID: uuid.UUID{}, | ||
}) | ||
if err != nil { | ||
// We are missing the display names, but that is not absolutely required. So just |
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.
Are we expecting an error here? I see we can do without it, but I wonder if we should instead of just presenting the error?
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'll return the error 👍
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.
Just unfortunate we can block api calls on this kind of error. Would be nice if we could return some sort of "warning" instead. I'll return an error though as that is what we usually do for failed db calls.
c757e52
to
45d4ff7
Compare
dfded68
to
1b3a9b7
Compare
45d4ff7
to
08e4131
Compare
e605d37
to
d23e335
Compare
08e4131
to
ce4b854
Compare
d23e335
to
9c1fda7
Compare
9c1fda7
to
0012858
Compare