Properly handle merges by the service, fix scenario test#5437
Merged
dkurepa merged 8 commits intodotnet:mainfrom Nov 6, 2025
Merged
Properly handle merges by the service, fix scenario test#5437dkurepa merged 8 commits intodotnet:mainfrom
dkurepa merged 8 commits intodotnet:mainfrom
Conversation
…t#5043) <!-- Link the GitHub or AzDO issue this pull request is associated with. Please copy and paste the full URL rather than using the dotnet/arcade-services# syntax --> dotnet#5030 --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: Adam <[email protected]>
Stop ForwardFlow updates if they'd overwrite manual PR changes (dotnet#5043)
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR refactors pull request handling logic to eliminate code duplication and improves test coverage for code flow scenarios. The changes extract common PR merge/close handling into a reusable method while adding a test step to verify PR commenting behavior.
- Extracted duplicate code handling merged/closed PRs into a new
HandleMergedOrClosedPrmethod - Added test step to re-trigger subscription for verifying PR comments after merge
- Minor indentation fix in test code
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| test/ProductConstructionService.ScenarioTests/ScenarioTests/ScenarioTests_CodeFlow.cs | Adds re-trigger of subscription in test to verify comment posting behavior after PR merge |
| src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestUpdater.cs | Refactors duplicate PR merge/close handling code into a new HandleMergedOrClosedPr method; adds unused import |
Comments suppressed due to low confidence (1)
src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestUpdater.cs:6
- The LibGit2Sharp namespace is imported but not used anywhere in this file. This import should be removed as it's unnecessary. The LibGit2Sharp types are abstracted behind the ILocalLibGit2Client interface used in this class.
using Maestro.Data.Models;
| pr.Url); | ||
| } | ||
|
|
||
| _logger.LogInformation("PR {url} has been manually {action}. Stopping tracking it", pr.Url, prInfo.Status.ToString().ToLowerInvariant()); |
There was a problem hiding this comment.
The log message says 'manually' but this is incorrect when the reason is DependencyFlowEventReason.AutomaticallyMerged. The log message should be conditional or changed to be accurate for both manual and automatic actions.
Suggested change
| _logger.LogInformation("PR {url} has been manually {action}. Stopping tracking it", pr.Url, prInfo.Status.ToString().ToLowerInvariant()); | |
| string mergeType = reason == DependencyFlowEventReason.AutomaticallyMerged ? "automatically" : "manually"; | |
| _logger.LogInformation("PR {url} has been {mergeType} {action}. Stopping tracking it", pr.Url, mergeType, prInfo.Status.ToString().ToLowerInvariant()); |
premun
reviewed
Nov 6, 2025
…enarioTests_CodeFlow.cs Co-authored-by: Přemek Vysoký <[email protected]>
premun
approved these changes
Nov 6, 2025
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.
#5263