Skip to content

Revert linking Forward Flown PRs after merging, do it on updates#5470

Merged
dkurepa merged 6 commits intodotnet:mainfrom
dkurepa:dkurepa/RevertLinkingPRsAfterClosing
Nov 13, 2025
Merged

Revert linking Forward Flown PRs after merging, do it on updates#5470
dkurepa merged 6 commits intodotnet:mainfrom
dkurepa:dkurepa/RevertLinkingPRsAfterClosing

Conversation

@dkurepa
Copy link
Copy Markdown
Member

@dkurepa dkurepa commented Nov 12, 2025

Reverts #5263 back to the original implementation

Copilot AI review requested due to automatic review settings November 12, 2025 15:20
Copy link
Copy Markdown
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 reverts changes from issue #5263, restoring the original implementation for linking forward-flown PRs. Instead of tagging PRs after they are merged, the system now tags them during PR updates.

Key Changes:

  • Removed post-merge PR tagging logic and restored update-time tagging
  • Cleaned up tracking of previous source SHA and related infrastructure
  • Simplified test scenarios to match reverted behavior

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
PullRequestUpdater.cs Removed HandleMergedOrClosedPr and TagForwardFlownPRs methods; inlined logic for handling merged/closed PRs; removed PreviousSourceSha tracking from code flow updates
InProgressPullRequest.cs Removed PreviousSourceSha property from the model
VmrForwardFlower.cs Restored PR linking logic to run during updates; moved regex patterns and PR extraction inline
IRemote.cs, IRemoteGitRepo.cs Removed GetCommitTitlesForRange interface method
Remote.cs Removed GetCommitTitlesForRange implementation; fixed method body formatting
GitHubClient.cs, AzureDevOpsClient.cs Removed GetCommitTitlesForRange implementations
CommentCollector.cs Removed Reset method from interface and implementation
GitRepoUtils.cs Deleted entire file (no longer needed)
ScenarioTests_CodeFlow.cs Simplified test to verify PR linking during updates instead of after merge
ScenarioTestBase.cs Removed PullRequestCleanupOperation enum and MergePullRequest method; simplified CleanUpPullRequestAfter to only close PRs
CodeFlowScenarioTestBase.cs Removed cleanupOperation and prState parameters from helper methods

Comment on lines 409 to 422
List<string> prNumbers = [];
foreach (var message in commitMessages)
{
foreach (var regex in regexes)
{
var match = regex.Match(message);
if (match.Success && match.Groups.Count > 1)
{
prNumbers.Add(match.Groups[1].Value);
break;
}
}
}

Copy link

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

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

This foreach loop immediately maps its iteration variable to another variable - consider mapping the sequence explicitly using '.Select(...)'.

Suggested change
List<string> prNumbers = [];
foreach (var message in commitMessages)
{
foreach (var regex in regexes)
{
var match = regex.Match(message);
if (match.Success && match.Groups.Count > 1)
{
prNumbers.Add(match.Groups[1].Value);
break;
}
}
}
List<string> prNumbers = commitMessages
.Select(message =>
{
var match = regexes
.Select(regex => regex.Match(message))
.FirstOrDefault(m => m.Success && m.Groups.Count > 1);
return match != null && match.Success && match.Groups.Count > 1
? match.Groups[1].Value
: null;
})
.Where(pr => pr != null)
.ToList()!;

Copilot uses AI. Check for mistakes.
premun
premun previously approved these changes Nov 13, 2025
Comment thread src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrForwardFlower.cs Outdated
@dkurepa dkurepa merged commit 3f2f513 into dotnet:main Nov 13, 2025
8 of 9 checks passed
@dkurepa dkurepa deleted the dkurepa/RevertLinkingPRsAfterClosing branch November 13, 2025 14:23
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