Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upInclude the Checks API in build status #2102
Conversation
smashwilson
added this to In progress
in Sprint : 4 April 2019 - 8 May 2019 : v0.29.0
via automation
…tView
codecov
bot
commented
Apr 26, 2019
•
Codecov Report
@@ Coverage Diff @@
## master #2102 +/- ##
==========================================
- Coverage 92.72% 92.53% -0.19%
==========================================
Files 207 212 +5
Lines 12053 12171 +118
Branches 1764 1779 +15
==========================================
+ Hits 11176 11263 +87
- Misses 877 908 +31
Continue to review full report at Codecov.
|
…hecks
|
Looks good! Excited to have more dotcom goodness in our extension with this addition Left a few of questions for ya. Unlikely that anything is blocking. |
smashwilson
added this to In progress
in Release : 9 May 2019 - 5 June 2019 : v0.30.0
via automation
Co-Authored-By: Katrina Uychaco <kuychaco@github.com>
…hecks
|
again, just a couple of questions; nothing blocking. Excited to ship this! |
| } | ||
|
|
||
| if (props.markdown && props.markdown !== state.lastMarkdown) { | ||
| return {html: renderMarkdown(props.markdown), lastMarkdown: props.markdown}; |
This comment has been minimized.
This comment has been minimized.
| @@ -9,6 +9,8 @@ export const LINE_ENDING_REGEX = /\r?\n/; | |||
| export const CO_AUTHOR_REGEX = /^co-authored-by. (.+?) <(.+?)>$/i; | |||
| export const PAGE_SIZE = 50; | |||
| export const PAGINATION_WAIT_TIME_MS = 100; | |||
| export const CHECK_SUITE_PAGE_SIZE = 10; | |||
| export const CHECK_RUN_PAGE_SIZE = 20; | |||
This comment has been minimized.
This comment has been minimized.
vanessayuenn
May 13, 2019
Contributor
What's the reasoning behind using these two numbers, instead of reusing the already defined PAGE_SIZE?
This comment has been minimized.
This comment has been minimized.
smashwilson
May 13, 2019
Author
Member
Oh, yeah. The page sizes we use in the query influences the rate-limiting cost associated with any given GraphQL query (as opposed to the number of results that are actually returned). It gets pretty complex, I'm not sure I fully understand it yet. But keeping our page sizes lower in cases when we probably don't need them gives us more "wiggle room" in our queries to add stuff later. I'm (unscientifically) betting here that most check suite and check run counts are below these numbers anyway.
What would be ideal, I think, is splitting out our page sizes for each resource type, and deriving a value from real dotcom stats, to target a generous 90% of real queries fitting in a single page (say).
smashwilson
merged commit 04db109
into
master
smashwilson commentedApr 26, 2019
•
edited
Please be sure to read the contributor's guide to the GitHub package before submitting any pull requests.
Requirements
Description of the Change
My goal is to account for check suites and check runs everywhere we display information about a PR or commit's status, to bring us in sync with the information that dotcom provides.
BuildStatusmodel to unify the information we derive from CheckSuite status and conclusions and build contexts.PullRequestDetailViewto show check suites and check runs.PullRequestStatusContextViewneeds to be adjusted to useBuildStatus.CheckSuiteViewandCheckRunViewcomponents render status rows from check suites and runs.PullRequestStatusesViewneeds to render the check suite accumulator to collect and render check suite data.summaryfield that may contain raw Markdown. Render and sanitize it.IssueishListViewto account for check results when rendering its status summary icon.BuildStatuswithin theIssueishmodel used by theIssueishListView.Screenshots
Alternate Designs
TODO
Benefits
Repositories configured to report advanced CI information with the Checks API will be accounted for correctly in the GitHub tab and pull request detail items.
Possible Drawbacks
If you have many check suites being reported on each commit, this may chew through your rate limit pretty quickly.
Applicable Issues
Fixes #1818.
Metrics
N/A
Tests
Recent commits in this repository have both legacy commit build statuses (from CodeCov) and check suite statuses (from Azure Pipelines). I've been using PRs within in this repo to verify that things look correct.
Documentation
N/A
Release Notes
User Experience Research (Optional)
N/A