Skip to content

Conversation

@georgy-gorelko
Copy link
Contributor

Pipeline ID Support

  • Added new --ado-pipeline-id option to support optional numeric pipeline IDs
  • Added comprehensive validation logic ensuring mutual exclusivity of pipeline name vs ID parameters
  • Added [GetPipelineId] helper method that returns provided ID directly or looks up ID by name
  • Updated both dry-run and regular execution paths to use the new resolution logic
  • Updated unit tests: Added tests for the new functionality and fixed existing tests to account for the new option

Key Features Implemented:

  1. Mutual Exclusion Validation: The command now properly validates that users specify either --ado-pipeline OR --ado-pipeline-id, but not both or neither

  2. Backward Compatibility: Existing functionality with --ado-pipeline continues to work exactly as before

  3. Enhanced Help Text: The help output clearly explains both options and their usage

  4. Robust Testing: Added comprehensive unit tests for all validation scenarios and functionality paths

  5. Improved Reliability: Using pipeline IDs eliminates the need for name-based lookups, making pipeline identification more reliable

Copilot AI review requested due to automatic review settings August 18, 2025 15:48
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enhances the RewirePipeline command to support optional pipeline ID vs pipeline name, adds comprehensive trigger preservation logic, and introduces a new pipeline testing framework.

Key changes include:

  • Added --ado-pipeline-id option as an alternative to --ado-pipeline
  • Implemented comprehensive trigger preservation logic with branch policy awareness
  • Added dry-run testing mode and new batch testing command
  • Enhanced pipeline rewiring to preserve original ADO trigger configurations

Reviewed Changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/ado2gh/Commands/RewirePipeline/RewirePipelineCommand.cs Added pipeline ID option, dry-run mode, and parameter validation
src/ado2gh/Commands/RewirePipeline/RewirePipelineCommandArgs.cs Added new properties for pipeline ID, dry-run mode, and timeout settings
src/ado2gh/Commands/RewirePipeline/RewirePipelineCommandHandler.cs Enhanced with pipeline ID resolution, dry-run testing, and trigger preservation
src/Octoshift/Services/AdoPipelineTriggerService.cs New service for managing complex pipeline trigger configuration during rewiring
src/Octoshift/Services/AdoApi.cs Added new methods for build operations and pipeline repository management
src/Octoshift/Services/PipelineTestService.cs New service for testing pipelines by temporarily rewiring them
src/ado2gh/Commands/TestPipelines/*.cs New batch testing command for testing multiple pipelines concurrently
Multiple test files Comprehensive test coverage for new functionality
Comments suppressed due to low confidence (1)

src/OctoshiftCLI.Tests/ado2gh/Commands/RewirePipeline/RewirePipelineCommandHandlerTests.cs:162

  • The URL is missing the base URL scheme and host. It should start with the full ADO service URL (e.g., 'https://dev.azure.com') instead of just '/'.
        await _handler.Handle(args);

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

@github-actions
Copy link

github-actions bot commented Aug 18, 2025

Unit Test Results

  1 files    1 suites   10m 24s ⏱️
942 tests 942 ✅ 0 💤 0 ❌
943 runs  943 ✅ 0 💤 0 ❌

Results for commit ce9c4c0.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@offbyone offbyone left a comment

Choose a reason for hiding this comment

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

It's really hard to review this in isolation, as it seems to include the changes in #1417 as well.

@georgy-gorelko
Copy link
Contributor Author

Need to finish #1412 and #1417

@boylejj boylejj requested a review from a team August 21, 2025 16:13
@georgy-gorelko
Copy link
Contributor Author

It's really hard to review this in isolation, as it seems to include the changes in #1417 as well.

All merged. @offbyone can you please take a look at?

georgy-gorelko and others added 2 commits September 24, 2025 09:30
…eCommandHandler

- Added try-catch blocks to handle HTTP request exceptions in AdoPipelineTriggerService.
- Implemented logging for 404 errors and other HTTP errors during pipeline retrieval.
- Updated RewirePipelineCommandHandler to throw exceptions for pipeline not found and lookup failures, with appropriate logging.
- Introduced unit tests for error handling scenarios in both AdoPipelineTriggerService and RewirePipelineCommandHandler.
@github-actions
Copy link

Code Coverage

Package Line Rate Branch Rate Complexity Health
gei 80% 72% 574
bbs2gh 83% 77% 653
ado2gh 72% 70% 712
Octoshift 83% 72% 1714
Summary 80% (7674 / 9537) 73% (1810 / 2491) 3653

@begonaguereca begonaguereca merged commit 6a4b4e6 into github:main Sep 24, 2025
26 of 27 checks passed
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.

3 participants