Skip to content

Conversation

@AshelyTC
Copy link
Contributor

@AshelyTC AshelyTC commented Feb 3, 2025

@AshelyTC AshelyTC requested a review from a team as a code owner February 3, 2025 17:50
Copy link
Contributor

@juxtin juxtin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice!

@juxtin
Copy link
Contributor

juxtin commented Feb 3, 2025

This should also help with GHES.

@AshelyTC AshelyTC merged commit 92129e5 into main Feb 3, 2025
6 checks passed
@AshelyTC AshelyTC deleted the ashelytc/server-url-fix branch February 3, 2025 19:46
} else if (error instanceof RequestError && error.status === 403) {
core.setFailed(
`Dependency review is not supported on this repository. Please ensure that Dependency graph is enabled along with GitHub Advanced Security on private repositories, see https://github.com/${github.context.repo.owner}/${github.context.repo.repo}/settings/security_analysis`
`Dependency review is not supported on this repository. Please ensure that Dependency graph is enabled along with GitHub Advanced Security on private repositories, see ${github.context.serverUrl}/${github.context.repo.owner}/${github.context.repo.repo}/settings/security_analysis`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @AshelyTC, are you sure we need to include context in the variable?

I'm asking because this page have it as github.server_url. We also use it in a different place in the code without context:

const serverUrl = new URL(
process.env['GITHUB_SERVER_URL'] ?? 'https://github.com'
)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I asked around: https://github.slack.com/archives/CNXKRE8CB/p1738613204710119 — it doesn't hurt to use github.context.serverUrl since it's coming from @actions/github, which is imported here.

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.

4 participants