-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 support to prerelease Integrations API #871
Conversation
lib/octokit/client/integrations.rb
Outdated
# | ||
# @return [<Sawyer::Resource>] An installation token | ||
def create_integration_installation_access_token(installation, options = {}) | ||
options = ensure_api_media_type(:integrations, options) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a pattern across the library, we avoid mutating the parameters. In other methods we usually use opts
as the variable for options after mutating them. In this case it would unlikely cause issues, but would bring everything into parity to follow the same pattern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in e0d5884
private | ||
|
||
def new_jwt_token | ||
private_pem = File.read(ENV['OCTOKIT_TEST_INTEGRATION_PEM_KEY']) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is throwing a nil error when the env variable is not defined. This could be changed to follow the rest of the library, using fetch inside a helper method in the spec helper. The fixture used for the CI could be used for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 7a5940c
@tarebyte Aside from my nitpicking, this looks good after a quick one-over. You seem to have a firm grasp of the vcr placeholders too and how to exploit them gainfully, 👍 |
1 similar comment
Thanks @tarebyte! |
This adds the early access Integrations API (https://developer.github.com/v3/integrations/) to Octokit.
Closes #832
/cc @keavy for Integration 👀