Update teams_discussions with ByID and BySlug endpoints #1426
Conversation
googlebot
added
the
cla: yes
There was a problem hiding this comment.
Thank you, @kadern0!
This is almost ready for a second review and merging, but please make the requested changes first.
Also, for future contributions, please indicate on the original issue that you are going to work on it so that others don't also start working on it and duplicating your work. Thank you!
github/teams_discussions.go
Outdated
| // Authenticated user must grant read:discussion scope. | ||
| // | ||
| // GitHub API docs: https://developer.github.com/v3/teams/discussions/#list-discussions | ||
| func (s *TeamsService) ListDiscussions(ctx context.Context, teamID int64, options *DiscussionListOptions) ([]*TeamDiscussion, *Response, error) { | ||
| u := fmt.Sprintf("teams/%v/discussions", teamID) | ||
| func (s *TeamsService) ListDiscussionsByID(ctx context.Context, orgID, teamID int64, options *DiscussionListOptions) ([]*TeamDiscussion, *Response, error) { |
There was a problem hiding this comment.
Let's please change options to opts to match what was recently done in #1417.
Here and throughout this PR.
| @@ -44,12 +44,12 @@ type DiscussionListOptions struct { | |||
| Direction string `url:"direction,omitempty"` | |||
There was a problem hiding this comment.
From the documentation, it looks like DiscussionListOptions also needs to embed a ListOptions struct to support pagination. Please add this.
There was a problem hiding this comment.
That's correct, I've updated it.
Replaced options by opts within function parameters Added ListOptions to DiscussionListOptions Signed-off-by: Pablo Caderno <[email protected]>
gmlewis
changed the title
|
@wesleimp - this could also be reviewed if you have time. |
|
Thank you, @wesleimp! |
Fixes #1424