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.

Sign up for GitHub

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

Jump to bottom

Rename id parameter to commentID in IssuesService comment methods. #886

Merged
merged 1 commit into from
Mar 29, 2018
Merged

Rename id parameter to commentID in IssuesService comment methods. #886

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions github/issues_comments.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ func (s *IssuesService) ListComments(ctx context.Context, owner string, repo str
// GetComment fetches the specified issue comment.
//
// GitHub API docs: https://developer.github.com/v3/issues/comments/#get-a-single-comment
func (s *IssuesService) GetComment(ctx context.Context, owner string, repo string, id int64) (*IssueComment, *Response, error) {
u := fmt.Sprintf("repos/%v/%v/issues/comments/%d", owner, repo, id)
func (s *IssuesService) GetComment(ctx context.Context, owner string, repo string, commentID int64) (*IssueComment, *Response, error) {
u := fmt.Sprintf("repos/%v/%v/issues/comments/%d", owner, repo, commentID)

req, err := s.client.NewRequest("GET", u, nil)
if err != nil {
Expand Down Expand Up @@ -118,10 +118,11 @@ func (s *IssuesService) CreateComment(ctx context.Context, owner string, repo st
}

// EditComment updates an issue comment.
// A non-nil comment.Body must be provided. Other comment fields should be left nil.
Copy link
Member Author

Choose a reason for hiding this comment

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

An additional step we can consider doing is checking if any of the comment fields other than Body are set (especially ID), and return an error with helpful text if so.

GitHub API documents that the only input field is body, and it is required. See https://developer.github.com/v3/issues/comments/#edit-a-comment:

image

A breaking API change to replace IssueComment parameter with a new IssueCommentEdit struct containing only the Body field is another option, but that's a breaking API change.

Copy link
Member Author

@dmitshur dmitshur Mar 29, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

This is an idea to consider, but I'm not ready to act on it yet. I'll leave it outside of the scope of this PR. If someone thinks it's worth doing, it can be sent and discussed in a followup PR.

//
// GitHub API docs: https://developer.github.com/v3/issues/comments/#edit-a-comment
func (s *IssuesService) EditComment(ctx context.Context, owner string, repo string, id int64, comment *IssueComment) (*IssueComment, *Response, error) {
u := fmt.Sprintf("repos/%v/%v/issues/comments/%d", owner, repo, id)
func (s *IssuesService) EditComment(ctx context.Context, owner string, repo string, commentID int64, comment *IssueComment) (*IssueComment, *Response, error) {
u := fmt.Sprintf("repos/%v/%v/issues/comments/%d", owner, repo, commentID)
req, err := s.client.NewRequest("PATCH", u, comment)
if err != nil {
return nil, nil, err
Expand All @@ -138,8 +139,8 @@ func (s *IssuesService) EditComment(ctx context.Context, owner string, repo stri
// DeleteComment deletes an issue comment.
//
// GitHub API docs: https://developer.github.com/v3/issues/comments/#delete-a-comment
func (s *IssuesService) DeleteComment(ctx context.Context, owner string, repo string, id int64) (*Response, error) {
u := fmt.Sprintf("repos/%v/%v/issues/comments/%d", owner, repo, id)
func (s *IssuesService) DeleteComment(ctx context.Context, owner string, repo string, commentID int64) (*Response, error) {
u := fmt.Sprintf("repos/%v/%v/issues/comments/%d", owner, repo, commentID)
req, err := s.client.NewRequest("DELETE", u, nil)
if err != nil {
return nil, err
Expand Down