Remove useGithubStorage from GHES version checker method
#1293
Conversation
Unit Test Results829 tests 829 ✅ 21s ⏱️ Results for commit 8774121. ♻️ This comment has been updated with latest results. |
begonaguereca
changed the title
useGithubStorage from GHES version checker method
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| @@ -407,7 +409,7 @@ | |||
| return; | |||
| } | |||
|
|
|||
| if (!shouldUseAzureStorage && !shouldUseAwsS3 && !args.UseGithubStorage) | |||
| if (!shouldUseAzureStorage && !shouldUseAwsS3 && !args.UseGithubStorage && !cloudCredentialsRequired) | |||
Check warning
Code scanning / CodeQL
Constant condition Warning
| var are_blob_credentials_required_based_on_ghes_version = await _ghesVersionChecker.AreBlobCredentialsRequired(args.GhesApiUrl); | ||
| var blobCredentialsRequired = !args.UseGithubStorage || are_blob_credentials_required_based_on_ghes_version; | ||
| var blobStorageUsed = args.UseGithubStorage || are_blob_credentials_required_based_on_ghes_version; |
There was a problem hiding this comment.
The previous logic was good. We need to always check the GHES version to be able to find out if it's > 3.8 or not. The only thing is that AreBlobCredentialsRequred doesn't need to know about the use github storage.
| @@ -396,7 +398,7 @@ private void ValidateGHESOptions(MigrateRepoCommandArgs args, bool cloudCredenti | |||
|
|
|||
| if (args.UseGithubStorage) | |||
| { | |||
| _log.LogWarning("Ignoring --use-github-storage flag because you are running GitHub Enterprise Server (GHES) 3.8.0 or later. The blob storage credentials configured in your GHES Management Console will be used instead."); | |||
| _log.LogWarning("Provding the --use-github-storage flag will supersede any credentials you have configured in your GitHub Enterprise Server (GHES) Management Console."); | |||
There was a problem hiding this comment.
I think that this logic is good. To be 100% though, I would recommend testing against GHES > 3.8 by configuring the creds in the management console + providing the --use-github-storage to verify.
|
This pull request simplifies the method calls to
AreBlobCredentialsRequiredby removing the boolean parameter that checks forgitHubOwnedStorage.While this produces the correct result, it is confusing for a reader. This PR refactors out that logic and adds the checks in the correct spots.
ThirdPartyNotices.txt(if applicable)