Support --target-api-url consistently on download-logs commands#1499
Support --target-api-url consistently on download-logs commands#1499
--target-api-url consistently on download-logs commands#1499Conversation
Unit Test Results 1 files 1 suites 10m 24s ⏱️ Results for commit 298fe37. ♻️ This comment has been updated with latest results. |
offbyone
force-pushed
the
o1/push-mvlyxwzkvtxx
branch
2 times, most recently
from
136a383 to
23f9b2c
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
offbyone
force-pushed
the
o1/push-mvlyxwzkvtxx
branch
from
23f9b2c to
0660e89
Compare
There was a problem hiding this comment.
Pull request overview
Updates download-logs commands to consistently support --target-api-url across ado2gh, bbs2gh, and gei, while preserving backward compatibility where needed.
Changes:
- Switched the shared
download-logsoption from--github-api-urlto--target-api-url. - Added
--github-api-urlas an alias for backward compatibility inado2ghandbbs2gh. - Updated unit tests and release notes to reflect the new option/alias behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/gei/Commands/DownloadLogs/DownloadLogsCommand.cs | Marks the command as sealed (no functional change to option behavior). |
| src/bbs2gh/Commands/DownloadLogs/DownloadLogsCommand.cs | Adds --github-api-url alias to the --target-api-url option for backward compatibility. |
| src/ado2gh/Commands/DownloadLogs/DownloadLogsCommand.cs | Adds --github-api-url alias to the --target-api-url option for backward compatibility. |
| src/OctoshiftCLI.Tests/bbs2gh/Commands/DownloadLogs/DownloadLogsCommandTests.cs | Updates option expectations and adds an alias-coverage test. |
| src/OctoshiftCLI.Tests/ado2gh/Commands/DownloadLogs/DownloadLogsCommandTests.cs | Updates option expectations and adds an alias-coverage test. |
| src/Octoshift/Commands/DownloadLogs/DownloadLogsCommandBase.cs | Renames the option token to --target-api-url and updates help text. |
| RELEASENOTES.md | Adds bullets describing the user-facing behavior changes. |
| public virtual Option<string> GithubApiUrl { get; } = new("--target-api-url") | ||
| { | ||
| Description = "Target GitHub API URL if not targeting github.com (default: https://api.github.com)." | ||
| Description = "Target GitHub API URL if not targeting github.com (default: https://api.github.com). Also accepts --github-api-url for backward compatibility." | ||
| }; |
There was a problem hiding this comment.
GithubApiUrl now defines the --target-api-url option, which is inconsistent with the TargetApiUrl naming used across other command bases (e.g., src/Octoshift/Commands/AbortMigration/AbortMigrationCommandBase.cs:33, src/Octoshift/Commands/CreateTeam/CreateTeamCommandBase.cs:27). Consider renaming the command/args property to TargetApiUrl to better match the established pattern and reduce confusion for future maintenance.
| @@ -1 +1,3 @@ | |||
|
|
|||
| - Fixed `ado2gh generate-script --download-migration-logs` to generate valid commands using `--target-api-url` that work with the `ado2gh download-logs` command | |||
There was a problem hiding this comment.
The first bullet says ado2gh generate-script --download-migration-logs was fixed to generate commands using --target-api-url, but this PR doesn’t change the generate-script implementation—only the download-logs option name/aliasing. Consider rewording this bullet to reflect the actual change (i.e., ado2gh download-logs now accepts --target-api-url / --github-api-url), to avoid implying behavior changed in generate-script.
Only the `gei` tree of commands had support for this flag, but the script generator for ado2gh and bbs2gh used it. This updates the flag support to use both
offbyone
force-pushed
the
o1/push-mvlyxwzkvtxx
branch
from
0660e89 to
79d8937
Compare
|
Only the
geitree of commands had support for this flag, but the script generator for ado2gh and bbs2gh used it.This updates the flag support to use both.
ThirdPartyNotices.txt(if applicable)Fixes the root cause of https://github.com/github/migration-friction/issues/4575