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

Use Octokit to generate installation access token #60

Merged
merged 2 commits into from
Jul 27, 2018

Conversation

ashmaroli
Copy link
Member

@ashmaroli ashmaroli commented Jul 27, 2018

Closes #‌21

In theory, this should be all that's required, though I do not see how one can reliably test this..

@pathawks
Copy link
Member

This doesn't quite close #21; we still have github/checks/* to transition to Octokit, but this is great 👍

This does output the following message to stderr, but I don't think that should be a problem:

WARNING: The preview version of the Integrations API is not yet suitable for production use.
You can avoid this message by supplying an appropriate media type in the 'Accept' request
header.

 * Remove unused requires
 * Continue to use `installation` variable
@pathawks pathawks merged commit b93568c into jekyll:master Jul 27, 2018
@ashmaroli
Copy link
Member Author

This doesn't quite close #21;

I see. Can you please edit the issue to have "tasks" in it..? One each for github/checks/* and another completed task for token..? Also, the has-pull-request label on the issue has to be removed..

@ashmaroli
Copy link
Member Author

I don't see a dedicated module to allow Octokit send requests to the Checks API, nor do I see an Issue / PR at the Octokit Repo to have one. So not sure what the best approach here would be.
/cc @parkr @mattr-

(FWIW, For my own test app, I was able to successfully patch Octokit to send requests to the Checks endpoints.)

@mattr-
Copy link
Member

mattr- commented Jul 28, 2018 via email

@ashmaroli
Copy link
Member Author

We haven’t updated Octokit yet for the new Checks API since it’s still in beta.

Thank you @mattr-. I had a hunch that this would be the reason..

Your best bet is to make post calls directly with Octokit.

@pathawks Would having the following code in Utterson be welcome..?

module Octokit
  class Client
    PREVIEW_HEADER = {
      :headers => {
        "Accept" => "application/vnd.github.antiope-preview+json"
      }
    }

    def create_check_run(repo, data)
      options = data.merge(PREVIEW_HEADER)
      post "#{Repository.path repo}/check-runs", options
    end

    def update_check_run(repo, id, data)
      options = data.merge(PREVIEW_HEADER)
      patch "#{Repository.path repo}/check-runs/#{id}", options
    end
  end
end

pathawks added a commit that referenced this pull request Aug 5, 2018
Octokit needs to be present on EC2 before the first time we run
`bundle update`. Because of this, we need to install it with our install
script. The Gemfile is just for running Jekyll, not API stuffs.

/cc #60
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.

3 participants