Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Conversation

@grokys
Copy link
Contributor

@grokys grokys commented May 4, 2018

Description

Use GraphQL to read the PR details in ModelService.GetPullRequest instead of using the ugly mix of REST and GraphQL that we were using before.

NOTE: The list of changed files in a PR are not exposed yet via GraphQL so we still need to use REST for these.

Implementation Notes

The query currently only reads the first 100 comments of the first 100 reviews for each PR. Will this be a problem? Paging with GraphQL is hard currently, we need to add support for automatic paging to Octokit.GraphQL.

Added a default constructor to PullRequestModel and made its Number property read/write so that all properties can be set from an initializer rather than requiring a mix of ctor parameters and initializers.

Testing

Nothing user-visible should change, or more precisely nothing should break, when interacting with the Pull Request detail view and PR reviews..

Depends on #1613

grokys and others added 7 commits April 17, 2018 18:32
Previously we were loading a flat list of PR review comments into the `PullRequestModel`. GraphQL nests review comments under their relevant review. Reflect this in our models as it makes more sense.
Changed files not yet exposed by GraphQL - we're still going to need to read that using REST for now.
Via REST as not yet exposed via GraphQL. Also read UpdatedAt.
@grokys grokys changed the title Read PR details using GraphQL WIP: Read PR details using GraphQL May 11, 2018
@grokys
Copy link
Contributor Author

grokys commented May 11, 2018

Marked this as WIP because now Octokit.GraphQL has auto-paging need to update it to use that.

@github github deleted a comment from drguthals Jun 7, 2018
@github github deleted a comment from drguthals Jun 7, 2018
@grokys
Copy link
Contributor Author

grokys commented Jun 13, 2018

Superseded by #1712

@grokys grokys closed this Jun 13, 2018
@grokys grokys deleted the refactor/pr-details-graphql branch June 13, 2018 09:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants