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

Can we deprecate @actions/github in favor of @octokit/action? #334

Closed
gr2m opened this issue Feb 4, 2020 · 11 comments
Closed

Can we deprecate @actions/github in favor of @octokit/action? #334

gr2m opened this issue Feb 4, 2020 · 11 comments
Labels
enhancement New feature or request

Comments

@gr2m
Copy link
Contributor

gr2m commented Feb 4, 2020

Describe the enhancement

It's rather confusing that both these modules exist. @actions/github is mostly wrapping @octokit/rest right now anyway. That's what @octokit/action does to, but with the benefit that I keep it up-to-date as part of the JavaScript Octokit SDKs.

Also Ocotkit comes with a plugin architecture. There is a growing list of scenarios that are nicely wrapped in a plugin so people don't have to re-create it. Here is an example: http://github.com/gr2m/octokit-plugin-create-pull-request

Code Snippet

Before

const github = require('@actions/github');

would become

const { Octokit } = require('@octokit/action');
const octokit = new Octokit()

Optionally add plugins and default settings

const { Octokit } = require('@octokit/action');
const { createPullRequest } = require('octokit-create-pull-request');
const MyOctokit = Octokit.plugin(createPullRequest).defaults({ userAgent: 'my-action/1.2.3' })
const octokit = new Octokit()

Additional information

Add any other context about the feature here.

@chrispat
Copy link
Member

Hi @gr2m,

Thank you for your interest in helping the GitHub Actions developer community.

One of our goals with maintaining the toolkit and this repo is providing a single place where the Actions developer community can go for help and to provide feedback to the product team. Further, given the nature of many Actions being more than running CI we consider interacting with the GitHub API to be a very core feature that the toolkit should offer.

With that in mind, would you consider making a pull request into the github module to enable direct interaction with the Octokit library so developers can make use of the plugin model?

@gr2m
Copy link
Contributor Author

gr2m commented Mar 10, 2020

I don't think one contradicts the other. I'd definitely keep the Actions toolkit, I'd just remove the part where you wrap @octokit/rest in https://github.com/actions/toolkit/tree/master/packages/github and instead update your documentation. Instead of

const github = require('@actions/github');

people would do

const { Octokit } = require('@octokit/action');
const octokit = new Octokit()

That is also the place where people could load custom plugins

const { Octokit } = require('@octokit/action');
const MyOctokit = Octokit.plugin([myplugin1, myplugin2])
const octokit = new Octokit()

The problem I see is that

  1. People report issue here that really are issues with Octokit. I am not monitoring this repository, and it's not as actively maintained as the Octokit repositories as far as I can tell.
  2. Some of the problems are caused by the way @octokit/action is implemented.
  3. I think it's confusing users to have two packages that basically do the same thing, both coming from GitHub

I can send a pull request to update @actions/github, but it would be a breaking change. You could just as well use that opportunity to point people to use Octokit which is GitHub's official SDK for their APIs. If there are features missing in @octokit/action, I'd be happy to add them there. And if people run into problems with using the API client, there is a clear place where they can report the bugs, instead of dividing it across two repositories

@eregon
Copy link

eregon commented Mar 10, 2020

One extra functionality that this action provides, and would need some way to migrate is:

const { GitHub, context } = require('@actions/github')

  const github = new GitHub(process.env.GITHUB_TOKEN)
  const { owner, repo } = context.repo
  let { data: releases } = await github.repos.listReleases({ owner, repo })

i.e. it's easy to know the owner and repo of the running action, which is convenient.

@gr2m
Copy link
Contributor Author

gr2m commented Mar 10, 2020

You can do

const [owner, repo] = process.env.GITHUB_REPOSITORY.split('/')

All that context does is reading out the environment variables. All the environment variables are documented.

I can also create a plugin which adds back that functionality for an easier migration

const core = require('@actions/core');
const { Octokit } = require('@octokit/rest')
const { actionContext } = require('octokit-plugin-action-context')

const ActionOctokit = Octokit.plugin(actionContext)
const octokit = new ActionOctokit({ auth: core.getInput('myToken') })
const { owner, repo } = octokit.context.repo

@gr2m
Copy link
Contributor Author

gr2m commented Mar 27, 2020

People keep running into issues because of @actions/github not being up-to-date, e.g. octokit/plugin-rest-endpoint-methods.js#60

@eregon
Copy link

eregon commented Mar 27, 2020

True, reading it out of process.env seems easy enough, would just need to be documented to migrate to @octokit/action.
👍 from me

@maxcountryman
Copy link

You can do

const [owner, repo] = process.env.GITHUB_REPOSITORY.split('/')

All that context does is reading out the environment variables. All the environment variables are documented.

I can also create a plugin which adds back that functionality for an easier migration

const { Octokit } = require('@octokit/action')
const { actionContext } = require('@octokit/plugin-action-context')

const ActionOctokit = Octokit.plugin(actionContext)
const octokit = new ActionOctokit()
const { owner, repo } = octokit.context.repo

Is the ref also available via process.env?

@gr2m
Copy link
Contributor Author

gr2m commented Apr 3, 2020

Yes: process.env.GITHUB_REF. See https://help.github.com/en/actions/configuring-and-managing-workflows/using-environment-variables#default-environment-variables

@gr2m
Copy link
Contributor Author

gr2m commented Apr 28, 2020

I created the Octokit plugin to ease the transition:
https://github.com/gr2m/octokit-plugin-action-context

The implementation does not have the checks if the GITHUB_EVENT_PATH or GITHUB_REPOSITORY environment variables exist, because I'm not aware of a case when these would be missing, do you?

The type for octokit.context.payload is not as good yet, but it could be much better if we would derived the type from the example payloads in octokit/webhooks: https://raw.githubusercontent.com/octokit/webhooks/master/index.json.

@billyvg
Copy link

billyvg commented Aug 18, 2020

is this still relevant now that @actions/github@3.x.x uses @octokit/core?

@gr2m
Copy link
Contributor Author

gr2m commented Aug 18, 2020

No I think we can close this now, cheers

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

No branches or pull requests

7 participants
@gr2m @maxcountryman @billyvg @eregon @chrispat and others