Remove useGithubStorage from GHES version checker method #1295
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ArinGhazarian
requested changes
Nov 12, 2024
src/gei/Commands/GenerateScript/GenerateScriptCommandHandler.cs
Outdated
Show resolved
Hide resolved
src/gei/Commands/GenerateScript/GenerateScriptCommandHandler.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Arin Ghazarian <[email protected]>
Co-authored-by: Arin Ghazarian <[email protected]>
Co-authored-by: Arin Ghazarian <[email protected]>
begonaguereca
changed the title
There was a problem hiding this comment.
So once the two comments are addressed it should be all good to go the only missing thing is that I forgot to mention is the INT tests for --use-github-storage are only testing a single part upload so it'd be ideal if there is one additional test for testing a multipart upload. Moving forward here are our options:
- Don’t add a test but run a manual functional test with a large archive > 1gb to make sure it 100% works (not recommended)
- Add a new INT test that uses a pre-existing archive > 100MiB (this can be a new INT test not the Basic one that doesn’t download the archive and uses an existing one and we just need to test
--use-github-storagescenario) (recommended) - if creating a > 100 MiB archive is not easily possible we can add a new test but change the archvie limit from 100 to say 10 MiB just to test the multipart upload.
Unit Test Results834 tests 834 ✅ 23s ⏱️ Results for commit edf8870. ♻️ This comment has been updated with latest results. |
Co-authored-by: Arin Ghazarian <[email protected]>
ArinGhazarian
approved these changes
Nov 13, 2024
src/OctoshiftCLI.Tests/gei/Commands/MigrateRepo/MigrateRepoCommandHandlerTests.cs
Outdated
Show resolved
Hide resolved
…mandHandlerTests.cs Co-authored-by: Arin Ghazarian <[email protected]>
ArinGhazarian
approved these changes
Nov 13, 2024
dylan-smith
reviewed
Nov 13, 2024
Co-authored-by: Dylan Smith <[email protected]>
ArinGhazarian
approved these changes
Nov 13, 2024
|
brianaj
approved these changes
Nov 15, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes https://github.ghe.com/github/octoshift/issues/9649!
This pull request simplifies the method calls to AreBlobCredentialsRequired by removing the boolean parameter that checks for gitHubOwnedStorage.
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)