-
Notifications
You must be signed in to change notification settings - Fork 13
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
make UserIdentifierInput fields pointers #301
Conversation
Codecov Report
@@ Coverage Diff @@
## main #301 +/- ##
=======================================
Coverage 76.75% 76.75%
=======================================
Files 49 49
Lines 3299 3299
=======================================
Hits 2532 2532
Misses 564 564
Partials 203 203
|
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!
@@ -797,7 +797,7 @@ func TestTeamAddMemberhip(t *testing.T) { | |||
team, _ := clientWithAlias.GetTeamWithAlias("example") | |||
newMembership := ol.TeamMembershipUserInput{ | |||
Role: "user", | |||
User: ol.UserIdentifierInput{Id: id1, Email: "[email protected]"}, | |||
User: ol.UserIdentifierInput{Id: &id1, Email: ol.NewString("[email protected]")}, |
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 "think" this should probably actually be using NewUserIdentifier("[email protected]")
- One thing i've been reading up about with idomatic go is that we should really push to use "constructors" for all our types so we can present better interfaces to "setup" objects and then also have a handle to do things like what we do in NewUserIdentifier
where we convert the "string" of e-mail into a *string
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.
Introducing NewUserIdentifier()
here would make the line read as:
User: NewUserIdentifier("[email protected]"),
but then we are getting a UserIdentifier
without an Id
Issues
#145 - partial fix
Changelog
Make
UserIdentifierInput
fields pointers to enable expectedomitempty
json tag behaviorchangie
entryTophatting
task test
- all tests pass