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

Support for the new GitHub Checks API #594

Merged
merged 12 commits into from May 16, 2018
Merged

Support for the new GitHub Checks API #594

merged 12 commits into from May 16, 2018

Conversation

orta
Copy link
Member

@orta orta commented May 11, 2018

WIP on support for using GH checks - #592 danger/danger#984

For a normal Danger (only) run, then Danger will need to know the following:

  • GitHub App ID
  • GitHub Installation ID for the Org
  • GitHub App Private Key

Definitely not a blocker, but certainly a bit non-trivial. This is intended to be opt-in for Danger, and probably the default for Peril.

I intend do to it by refactoring the commenting aspect of the GitHub platform to either be the issue comment we have now, or via the checks API exclusively.

@DangerCI
Copy link

DangerCI commented May 11, 2018

New dependencies added: jsonwebtoken and @types/jsonwebtoken.

jsonwebtoken

Author: auth0

Description: JSON Web Token implementation (symmetric and asymmetric)

Homepage: https://github.com/auth0/node-jsonwebtoken#readme

Createdalmost 5 years ago
Last Updatedabout 1 month ago
LicenseMIT
Maintainers8
Releases73
Direct Dependenciesjws, lodash.includes, lodash.isboolean, lodash.isinteger, lodash.isnumber, lodash.isplainobject, lodash.isstring, lodash.once, ms and xtend
Keywordsjwt
This README is too long to show.

@types/jsonwebtoken

Author: Unknown

Description: TypeScript definitions for jsonwebtoken

Homepage: http://npmjs.com/package/@types/jsonwebtoken

Createdalmost 2 years ago
Last Updated11 days ago
LicenseMIT
Maintainers1
Releases23
Direct Dependencies@types/node
README

Installation

npm install --save @types/jsonwebtoken

Summary

This package contains type definitions for jsonwebtoken (https://github.com/auth0/node-jsonwebtoken).

Details

Files were exported from https://www.github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/jsonwebtoken

Additional Details

  • Last updated: Mon, 30 Apr 2018 16:18:31 GMT
  • Dependencies: node
  • Global values: none

Credits

These definitions were written by Maxime LUCE https://github.com/SomaticIT, Daniel Heim https://github.com/danielheim, Brice BERNARD https://github.com/brikou, Veli-Pekka Kestilä https://github.com/vpk.

Generated by 🚫 dangerJS

@orta orta changed the title Refactors the comment sides of the GH Platform to work via another object [WIP] Support for the new GitHub Checks API May 13, 2018
@orta
Copy link
Member Author

orta commented May 13, 2018

Sidenote: it's possible that this next commit may break using the API with enterprise apps

@orta
Copy link
Member Author

orta commented May 13, 2018

Somewhat blocked by https://github.com/octokit/rest.js/pull/866 and https://github.com/octokit/rest.js/pull/864 for the mo' - think I have all the data set up, will need to handle the API stuff once they're good

@sunshinejr
Copy link
Member

sunshinejr commented May 13, 2018

This is great @orta, thanks for working on this so quickly! Really excited about that one, the integration looks awesome

@orta orta force-pushed the checks branch 3 times, most recently from 8b87498 to 6105752 Compare May 15, 2018 03:26
@orta
Copy link
Member Author

orta commented May 15, 2018

Got my first check working 👍

Was trivial work with octokit @gr2m

- echo "This is the real `danger ci` run on this repo"
- DEBUG="*" danger ci --verbose

# If the PR is odd then run using issue commenting, else use checks

Choose a reason for hiding this comment

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

✨✨ a/b testing ✨✨

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks man, yeah, I was thinking for a while on how I can ensure this is regularly integrated properly - love this hack, I've not yet got it working but I will get it eventually

@fbartho
Copy link
Member

fbartho commented May 15, 2018

This is really exciting. Too bad it's going to require such a tricky integration process :(

@orta
Copy link
Member Author

orta commented May 15, 2018

Given the security model of GitHub Apps I can just give out both an app ID, and a private key with access to checks, people will only need to install it on their org and give danger the install ID. Could be less work in general.

@orta
Copy link
Member Author

orta commented May 16, 2018

I went one better, it's way simpler to set up Danger now:

No faffing with making a new user, or new app.

@gr2m
Copy link
Contributor

gr2m commented May 16, 2018

Somewhat blocked by octokit/rest.js#866 and octokit/rest.js#864 for the mo'

No mo': https://github.com/octokit/rest.js/releases

@orta
Copy link
Member Author

orta commented May 16, 2018

It works: https://github.com/danger/danger-js/pull/594/checks !

@orta orta changed the title [WIP] Support for the new GitHub Checks API Support for the new GitHub Checks API May 16, 2018
@orta
Copy link
Member Author

orta commented May 16, 2018

Alright, I'm shipping this so I can try it out in Peril.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants