-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Inject GitHub host to be able to clone from another GitHub instance #922
Conversation
…o differ from the Actions workflow host
8e2ca23
to
385d3aa
Compare
__test__/git-auth-helper.test.ts
Outdated
@@ -778,7 +778,8 @@ async function setup(testName: string): Promise<void> { | |||
sshKnownHosts: '', | |||
sshStrict: true, | |||
workflowOrganizationId: 123456, | |||
setSafeDirectory: true | |||
setSafeDirectory: true, | |||
githubServerUrl: undefined |
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.
can we have a test with githubServerUrl
set to some URL?
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.
Added tests to the branch and refactored the one that was there to use common code for that test
src/octokit-helper.ts
Outdated
|
||
export type OctokitOptions = { | ||
baseUrl?: string | ||
userAgent?: string |
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.
anything set this userAgent
?
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.
Yes the ref-helper.ts
injects this currently, https://github.com/peter-murray/checkout/blob/server-url-injection/src/ref-helper.ts#L250
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.
@peter-murray can you please run npm run format
and push the changes?
Otherwise, I don't see any glaring issues with this change.
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.
Minor feedback, otherwise LGTM
3e002a4
to
0ff8af6
Compare
These changes allow for the base URL of another GitHub instance to be provided to the action so that a workflow running on a GHES server can clone a repository from another GHES server or github.com.
These changes maintain the existing default behaviour whilst allowing for a user to inject a URL to alternate GitHub instance and have been validated using an actions runner with and without a git command line client (i.e. git command line clone and HTTP cloning) have been tested.