Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement target-api-url for generate-script commands #1223

Merged
merged 15 commits into from
Feb 9, 2024
Merged

Conversation

brianaj
Copy link
Collaborator

@brianaj brianaj commented Feb 9, 2024

Implement target-api-url for generate-script commands that leverages prev work for migrate-repo download-logs wait-for-migration.

last one for #1094 and https://github.ghe.com/github/octoshift/issues/7956

Note: These unit tests were extremely tricky for me, I did some functional testing in original issue but spending over a day on unit tests is my limit.

  • Did you write/update appropriate tests
  • Release notes updated (if appropriate)
  • Appropriate logging output
  • Issue linked
  • Docs updated (or issue created)
  • New package licenses are added to ThirdPartyNotices.txt (if applicable)

@brianaj brianaj changed the title 1094 last one Implement target-api-url for generate-script Feb 9, 2024
@brianaj brianaj changed the title Implement target-api-url for generate-script Implement target-api-url for generate-script commands Feb 9, 2024
@brianaj brianaj marked this pull request as ready for review February 9, 2024 05:38
Copy link

github-actions bot commented Feb 9, 2024

Unit Test Results

810 tests   810 ✅  21s ⏱️
  1 suites    0 💤
  1 files      0 ❌

Results for commit 54158a7.

♻️ This comment has been updated with latest results.

@brianaj brianaj requested a review from ArinGhazarian February 9, 2024 19:34
Copy link
Collaborator

@ArinGhazarian ArinGhazarian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good just left some comments to use our HasValue() extension method instead of !string.IsNullOrEmpty()

@brianaj brianaj merged commit 1ab43a4 into main Feb 9, 2024
30 checks passed
@brianaj brianaj deleted the 1094-last-one branch February 9, 2024 22:01
Copy link

github-actions bot commented Feb 9, 2024

Code Coverage

Package Line Rate Branch Rate Complexity Health
Octoshift 87% 76% 1272
gei 79% 70% 519
bbs2gh 78% 73% 651
ado2gh 84% 78% 627
Summary 83% (6821 / 8172) 75% (1534 / 2044) 3069

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants