-
Notifications
You must be signed in to change notification settings - Fork 457
Replace @action/github
dependency with latest Octokit
#614
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
base: main
Are you sure you want to change the base?
Conversation
"module": "Node16", | ||
"moduleResolution": "node16", |
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.
It appears all of the newer Octokit libraries require this - https://github.com/octokit/action.js/blob/25f537f58b9d8ee779175a9601f0958b1b2ce6ac/README.md#L33-L37
"author": "GitHub", | ||
"license": "MIT", | ||
"main": "dist/index.js", | ||
"type": "module", |
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.
I'm fairly certain this was required when updating Octokit, I don't think it should impact much since this action isn't consumed as a library.
My one concern is that it somehow impacts the custom require
functionality this action implements, but integration tests should catch that.
dist/index.js
Outdated
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.
I'm not entirely sure why the dist is so bloated now
id: my-script | ||
with: | ||
result-encoding: string | ||
retries: 3 | ||
script: | | ||
github.rest.issues.get({ | ||
octokit.rest.issues.get({ |
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 replaces the dependency of
@actions/github
with a more direct dependency on Octokit via@octokit/action
- https://github.com/octokit/action.js.@actions/github
is on a very old version of Octokit and is preventing users of this action from using the latest GitHub APIs in their scripts:Using Octokit in a more direct manner will allow this action to keep up date with dependencies in a more timely manner going forward.
This will likely break some user scripts, both from "expected" breaking changes from a multi-version bump of Octokit and unexpected breakages from new or different behavior in
@octokit/action
or this action that we're not correctly replicating from@actions/github
. My hope is that integration tests will catch the more significant scenarios, but we should expect some bugs with the v8 version of this action once it's released - #556.