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.

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

Add Author Association for comments related models #1877

Merged
merged 3 commits into from
Nov 7, 2018

Conversation

mirsaeedi
Copy link
Contributor

It seems that GitHub has recently added a new field called author_association to comment related response models such as Issue Comments, Pull Request Review Comments, and Pull Request Reviewers. This field shows how an author is related to a repo.

The proposed change adds a new enum named AuthorAssociate, changes constructors in affected models, and lastly adds a new attribute called AuthorAssociate to related models.

@ryangribble
Copy link
Contributor

Hey @mirsaeedi sorry I didn't have a chance to look at this yet... The documentation you linked to is for graphQL API, but this field is also being returned in REST API responses?

In the constructors can you remove the =null parts as we don't use default/optional parameters anywhere else so I'd like to stay consistent with that

@mirsaeedi
Copy link
Contributor Author

Hey @ryangribble, yeah as you can see from the links, this field is returned in REST api responses.

I will remove the optional parameter. I put it in this way to not break existing codes.

@ryangribble
Copy link
Contributor

Oh yes sorry it only looked at the first link and not the others!

I understand the attempt to make it non breaking but these are only response classes which nobody would really be creating anyway (except in mocked tests) and just from a consistency point of view we don't do this anywhere else so I'd rather not do it now

@mirsaeedi
Copy link
Contributor Author

it's done. @ryangribble

@ryangribble
Copy link
Contributor

is this field provided in every API response that returns comments? If not, it should be nullable (so it can be null when the API doesnt provide it)

@ryangribble
Copy link
Contributor

ryangribble commented Nov 7, 2018

release_notes: Added AuthorAssociation field to IssueComment, PullRequestReview and PullRequestReviewComment response models

@ryangribble
Copy link
Contributor

Thanks @mirsaeedi !

LGTM

@ryangribble ryangribble merged commit eb9c112 into octokit:master Nov 7, 2018
@nickfloyd nickfloyd added Type: Feature New feature or request and removed category: feature labels Oct 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants