-
Notifications
You must be signed in to change notification settings - Fork 77
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
CLOUDP-276151: Migrate Assigned Teams Translation Layer #1850
CLOUDP-276151: Migrate Assigned Teams Translation Layer #1850
Conversation
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.
LGTM! Good job!
g.Go(func() error { | ||
workflowCtx.Log.Debugf("removing user %s from team %s", user.UserID, teamID) | ||
err := r.teamsService.RemoveUser(workflowCtx.Context, workflowCtx.OrgID, teamID, user.UserID) | ||
return err |
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.
non-blocking super small nit:
return err | |
return r.teamsService.RemoveUser(workflowCtx.Context, workflowCtx.OrgID, teamID, user.UserID) |
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.
For the sake of pushing this through merge and avoiding sitting through another set of flakey test runs, I'll dismiss this nit.
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.
Great job @cveticm! 🔥 LGTM 👍
CLOUDP-276151: Implementation of translation layer for assigned teams.