persist core.sshCommand for submodules #184
Conversation
| @@ -437,15 +439,16 @@ describe('git-auth-helper tests', () => { | |||
| } | |||
| ) | |||
|
|
|||
| const configureSubmoduleAuth_configuresTokenWhenPersistCredentialsTrueAndSshKeyNotSet = | |||
| 'configureSubmoduleAuth configures token when persist credentials true and SSH key not set' | |||
| const configureSubmoduleAuth_configuresSubmodulesWhenPersistCredentialsFalseAndSshKeyNotSet = | |||
There was a problem hiding this comment.
some of the tests for configureSubmoduleAuth were redundant so i collapsed based on input combinations:
persist-credentials: false,ssh-key: ''persist-credentials: false,ssh-key: 'set'persist-credentials: true,ssh-key: ''persist-credentials: true,ssh-key: 'set'
There was a problem hiding this comment.
Each test just tests the submoduleForeach calls so it made sense to collapse.
| expect(mockSubmoduleForeach.mock.calls[0][0] as string).toMatch( | ||
| /unset-all.*insteadOf/ | ||
| ) | ||
| expect(mockSubmoduleForeach.mock.calls[2][0]).toMatch(/core\.sshCommand/) |
There was a problem hiding this comment.
assert core.sshCommand was added
| @@ -5122,6 +5122,7 @@ class GitAuthHelper { | |||
| this.tokenConfigKey = `http.https://${HOSTNAME}/.extraheader`; | |||
There was a problem hiding this comment.
dont review index.js (generated)
| @@ -57,6 +57,16 @@ export async function getSource(settings: IGitSourceSettings): Promise<void> { | |||
| core.info( | |||
| `To create a local Git repository instead, add Git ${gitCommandManager.MinimumGitVersion} or higher to the PATH` | |||
| ) | |||
| if (settings.submodules) { | |||
There was a problem hiding this comment.
fail when fallback to REST API and submodules or ssh-key is set
There was a problem hiding this comment.
Shouldn't we do this before logging 'The repository will be downloaded using the GitHub REST API'
Since errors propagate up as annotations. Consider: "Git ${gitCommandManager.MinimumGitVersion} version or greater not found in path. Input 'submodules' not supported when falling back to download using the GitHub REST API"
Same for ssh key
There was a problem hiding this comment.
I really like failing as an error flow here 👍 , as opposed to trying to download without submodules or without using the ssh key
There was a problem hiding this comment.
Updated. To make the error actionable. I switched the order a little bit from the suggestion, to lead with the error message followed by suggested action.
ericsciple
force-pushed
the
users/ericsciple/m167sub
branch
from
95fd335
to
95471c2
Compare
No description provided.