Fix ghe.com source handling in migrate-repo - #1606
Draft
brianaj wants to merge 7 commits into
Draft
Conversation
migrate-repo
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes ghe.com source URL registration and storage-warning behavior in migrate-repo.
Changes:
- Extracts tenant base URLs from GHES/ghe.com API URLs.
- Registers ghe.com migration sources with their tenant URL.
- Adds source-specific storage guidance and tests.
Show a summary per file
| File | Description |
|---|---|
RELEASENOTES.md |
Documents user-facing fixes. |
src/Octoshift/Extensions/StringExtensions.cs |
Adds base URL extraction. |
src/Octoshift/Services/GithubApi.cs |
Supports custom migration source URLs. |
src/gei/Commands/MigrateRepo/MigrateRepoCommandHandler.cs |
Applies tenant URLs and source-specific warnings. |
src/OctoshiftCLI.Tests/StringExtensionsTests.cs |
Tests URL extraction. |
src/OctoshiftCLI.Tests/Octoshift/Services/GithubApiTests.cs |
Tests custom source URL payloads. |
src/OctoshiftCLI.Tests/gei/Commands/MigrateRepo/MigrateRepoCommandHandlerTests.cs |
Tests ghe.com handler behavior. |
Review details
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Suppressed comments (1)
src/gei/Commands/MigrateRepo/MigrateRepoCommandHandler.cs:437
- This guidance is inaccurate for archive URLs: those URLs are already hosted, and argument validation rejects
--use-github-storagewith only--git-archive-url/--metadata-archive-url(MigrateRepoCommandArgs.cs:98-107). The flag is applicable to on-disk archive paths, not archive URLs. Please remove that case from the warning.
_log.LogWarning("The --use-github-storage flag is not required when migrating from GitHub Enterprise Cloud with data residency (ghe.com); the migration API handles storage. Only pass it if you are supplying your own archives via archive URLs or on-disk archive paths.");
- Files reviewed: 7/7 changed files
- Comments generated: 3
- Review effort level: Balanced
Unit Test Results 1 files 1 suites 25s ⏱️ Results for commit bc569e5. |
This file contains hidden or 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
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.
Fixes #1604
What
Two small fixes for migrations that use a GitHub Enterprise Cloud with data residency (ghe.com) source via
--github-source-api-url:https://<tenant>.ghe.com) instead of alwayshttps://github.com. This is cosmetic and did not affect migrations.--use-github-storageis passed, the CLI now notes the flag isn't required for ghe.com sources unless you are supplying your own archives.Why
Reported in #1604: the source URL was hardcoded to
github.com, and the GHES-specific storage warning surfaced on a source where it doesn't apply.Changes
ExtractGitHubBaseUrlstring extension that handles both GHES (.../api/v3) and ghe.com (api.<host>) URL shapes; replaced the private helper in the migrate-repo handler.CreateGhecMigrationSourcenow accepts an optional source URL (via an overload); the handler passes the tenant base URL for ghe.com sources.Testing
CreateGhecMigrationSource, and the migrate-repo handler. Full suite passing.--use-github-storage: the migration source uses the tenant URL and the storage warning behaves correctly in both cases.Release notes
Added two entries to
RELEASENOTES.md.ThirdPartyNotices.txt(if applicable)