-
Notifications
You must be signed in to change notification settings - Fork 141
Add client-id input and deprecate app-id
#353
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?
Changes from all commits
e9da442
8204e76
37f42c5
ea0d315
136ba78
188519e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,7 +18,10 @@ if (!process.env.GITHUB_REPOSITORY_OWNER) { | |
| async function run() { | ||
| ensureNativeProxySupport(); | ||
|
|
||
| const appId = core.getInput("app-id"); | ||
| const clientId = core.getInput("client-id") || core.getInput("app-id"); | ||
| if (!clientId) { | ||
| throw new Error("Either 'client-id' or 'app-id' input must be set"); | ||
| } | ||
|
Comment on lines
+21
to
+24
|
||
| const privateKey = core.getInput("private-key"); | ||
| const owner = core.getInput("owner"); | ||
| const repositories = core | ||
|
|
@@ -32,7 +35,7 @@ async function run() { | |
| const permissions = getPermissionsFromInputs(process.env); | ||
|
|
||
| return main( | ||
| appId, | ||
| clientId, | ||
| privateKey, | ||
| owner, | ||
| repositories, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| import { DEFAULT_ENV, test } from "./main.js"; | ||
|
|
||
| // Verify `main` accepts a GitHub App client ID via the `client-id` input | ||
| await test( | ||
| () => {}, | ||
| { | ||
| ...DEFAULT_ENV, | ||
| "INPUT_CLIENT-ID": "Iv1.0123456789abcdef", | ||
| "INPUT_APP-ID": "", | ||
| } | ||
| ); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| import { DEFAULT_ENV } from "./main.js"; | ||
|
|
||
| for (const [key, value] of Object.entries({ | ||
| ...DEFAULT_ENV, | ||
| "INPUT_CLIENT-ID": "", | ||
| "INPUT_APP-ID": "", | ||
| })) { | ||
| process.env[key] = value; | ||
| } | ||
|
|
||
| // Log only the error message, not the full stack trace, because the stack | ||
| // trace contains environment-specific paths and ANSI codes that differ | ||
| // between local and CI environments. | ||
| const _error = console.error; | ||
| console.error = (err) => _error(err?.message ?? err); | ||
|
|
||
| // Verify `main` exits with an error when neither `client-id` nor `app-id` is set. | ||
| const { default: promise } = await import("../main.js"); | ||
| await promise; | ||
| process.exitCode = 0; |
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.
The JSDoc and parameter name (
clientId) suggest this argument is always a GitHub App Client ID, but callers pass eitherclient-idor legacyapp-idhere. Tweaking the docs/parameter name to reflect that it can be either (e.g.,appIdOrClientId) would make intent clearer, especially since it’s mapped intocreateAppAuth({ appId: ... }).