WARNING: THIS SITE IS A MIRROR OF GITHUB.COM / IT CANNOT LOGIN OR REGISTER ACCOUNTS / THE CONTENTS ARE PROVIDED AS-IS / THIS SITE ASSUMES NO RESPONSIBILITY FOR ANY DISPLAYED CONTENT OR LINKS / IF YOU FOUND SOMETHING MAY NOT GOOD FOR EVERYONE, CONTACT ADMIN AT ilovescratch@foxmail.com
Skip to content

Commit c5c05e1

Browse files
premunpremun
authored
Revert "Improve darc update-subscription by allowing to specify policy checks" (#4689)
Reverts #4680 This reverts commit 15c4f8e.
1 parent 7cfcd52 commit c5c05e1

File tree

12 files changed

+80
-218
lines changed

12 files changed

+80
-218
lines changed

‎src/Maestro/Maestro.MergePolicies/BackFlowMergePolicy.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ private static List<string> CalculateConfigurationErrors(
120120

121121
public class BackFlowMergePolicyBuilder : IMergePolicyBuilder
122122
{
123-
public string Name => MergePolicyConstants.CodeflowMergePolicyName;
123+
public string Name => MergePolicyConstants.BackFlowMergePolicyName;
124124

125125
public Task<IReadOnlyList<IMergePolicy>> BuildMergePoliciesAsync(MergePolicyProperties properties, PullRequestUpdateSummary pr)
126126
{

‎src/Maestro/Maestro.MergePolicies/ForwardFlowMergePolicy.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ private static bool TryCreateCommitShaDictionaryFromSourceManifest(SourceManifes
128128

129129
public class ForwardFlowMergePolicyBuilder : IMergePolicyBuilder
130130
{
131-
public string Name => MergePolicyConstants.CodeflowMergePolicyName;
131+
public string Name => MergePolicyConstants.ForwardFlowMergePolicyName;
132132

133133
public Task<IReadOnlyList<IMergePolicy>> BuildMergePoliciesAsync(MergePolicyProperties properties, PullRequestUpdateSummary pr)
134134
{

‎src/Maestro/Maestro.MergePolicies/MergePolicyServiceCollectionExtensions.cs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,6 @@ public static IServiceCollection AddMergePolicies(this IServiceCollection servic
1414
services.AddTransient<IMergePolicyBuilder, DontAutomergeDowngradesMergePolicyBuilder>();
1515
services.AddTransient<IMergePolicyBuilder, StandardMergePolicyBuilder>();
1616
services.AddTransient<IMergePolicyBuilder, ValidateCoherencyMergePolicyBuilder>();
17-
services.AddTransient<IMergePolicyBuilder, BackFlowMergePolicyBuilder>();
18-
services.AddTransient<IMergePolicyBuilder, ForwardFlowMergePolicyBuilder>();
1917
return services;
2018
}
2119
}

‎src/Maestro/Maestro.MergePolicies/StandardMergePolicy.cs‎

Lines changed: 40 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,57 +1,66 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
using Maestro.MergePolicyEvaluation;
5+
using Newtonsoft.Json.Linq;
46
using System;
57
using System.Collections.Generic;
6-
using System.Linq;
78
using System.Threading.Tasks;
8-
using Maestro.MergePolicyEvaluation;
9-
using Newtonsoft.Json.Linq;
109

1110
namespace Maestro.MergePolicies;
1211

1312
public class StandardMergePolicyBuilder : IMergePolicyBuilder
1413
{
15-
private static readonly IReadOnlyList<string> s_standardGitHubIgnoreChecks = [
16-
"WIP",
17-
"license/cla",
18-
"auto-merge.config.enforce",
19-
"Build Analysis"
20-
];
21-
private static readonly IReadOnlyList<string> s_standardAzureDevOpsIgnoreChecks = [
22-
"Comment requirements",
23-
"Minimum number of reviewers",
24-
"auto-merge.config.enforce",
25-
"Work item linking"
26-
];
14+
private static readonly MergePolicyProperties s_standardGitHubProperties;
15+
private static readonly MergePolicyProperties s_standardAzureDevOpsProperties;
2716

2817
public string Name => MergePolicyConstants.StandardMergePolicyName;
2918

30-
private static IEnumerable<string> GetStandardIgnoreChecks(string prUrl)
19+
static StandardMergePolicyBuilder()
3120
{
32-
if (prUrl.Contains("github.com"))
21+
s_standardGitHubProperties = new MergePolicyProperties(new Dictionary<string, JToken>
3322
{
34-
return s_standardGitHubIgnoreChecks;
35-
}
36-
else if (prUrl.Contains("dev.azure.com"))
23+
{
24+
MergePolicyConstants.IgnoreChecksMergePolicyPropertyName,
25+
new JArray(
26+
"WIP",
27+
"license/cla",
28+
"auto-merge.config.enforce",
29+
"Build Analysis"
30+
)
31+
},
32+
});
33+
34+
s_standardAzureDevOpsProperties = new MergePolicyProperties(new Dictionary<string, JToken>
3735
{
38-
return s_standardAzureDevOpsIgnoreChecks;
39-
}
40-
throw new NotImplementedException($"Unknown PR repo URL: {prUrl}");
36+
{
37+
MergePolicyConstants.IgnoreChecksMergePolicyPropertyName,
38+
new JArray(
39+
"Comment requirements",
40+
"Minimum number of reviewers",
41+
"auto-merge.config.enforce",
42+
"Work item linking"
43+
)
44+
},
45+
});
4146
}
4247

4348
public async Task<IReadOnlyList<IMergePolicy>> BuildMergePoliciesAsync(MergePolicyProperties properties, PullRequestUpdateSummary pr)
4449
{
4550
string prUrl = pr.Url;
46-
MergePolicyProperties standardProperties = new(new Dictionary<string, JToken>
51+
MergePolicyProperties standardProperties;
52+
if (prUrl.Contains("github.com"))
4753
{
48-
{
49-
MergePolicyConstants.IgnoreChecksMergePolicyPropertyName,
50-
new JArray(GetStandardIgnoreChecks(prUrl)
51-
.Concat(properties.Get<IEnumerable<string>>(MergePolicyConstants.IgnoreChecksMergePolicyPropertyName) ?? [])
52-
.Distinct())
53-
}
54-
});
54+
standardProperties = s_standardGitHubProperties;
55+
}
56+
else if (prUrl.Contains("dev.azure.com"))
57+
{
58+
standardProperties = s_standardAzureDevOpsProperties;
59+
}
60+
else
61+
{
62+
throw new NotImplementedException("Unknown pr repo url");
63+
}
5564

5665
var policies = new List<IMergePolicy>();
5766
policies.AddRange(await new AllChecksSuccessfulMergePolicyBuilder().BuildMergePoliciesAsync(standardProperties, pr));

‎src/Maestro/Maestro.MergePolicyEvaluation/MergePolicyConstants.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ public class MergePolicyConstants
1717

1818
public const string MaestroMergePolicyDisplayName = "Maestro auto-merge";
1919

20-
public const string CodeflowMergePolicyName = "Codeflow consistency check";
20+
public const string ForwardFlowMergePolicyName = "Codeflow consistency check";
21+
public const string BackFlowMergePolicyName = "Codeflow consistency check";
2122
}
2223

‎src/Microsoft.DotNet.Darc/Darc/Models/PopUps/MergePoliciesPopUpHelpers.cs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,7 @@ public static bool ValidateMergePolicies(List<MergePolicy> mergePolicies, ILogge
2323
{
2424
foreach (MergePolicy policy in mergePolicies)
2525
{
26-
if (policy.Name.Equals(MergePolicyConstants.AllCheckSuccessfulMergePolicyName, StringComparison.OrdinalIgnoreCase) ||
27-
policy.Name.Equals(MergePolicyConstants.StandardMergePolicyName, StringComparison.OrdinalIgnoreCase))
26+
if (policy.Name.Equals(MergePolicyConstants.AllCheckSuccessfulMergePolicyName, StringComparison.OrdinalIgnoreCase))
2827
{
2928
// Should either have no properties, or one called "ignoreChecks"
3029
if (policy.Properties != null &&
@@ -36,7 +35,7 @@ public static bool ValidateMergePolicies(List<MergePolicy> mergePolicies, ILogge
3635
return false;
3736
}
3837
}
39-
else if (policy.Name.Equals(MergePolicyConstants.CodeflowMergePolicyName, StringComparison.OrdinalIgnoreCase) ||
38+
else if (policy.Name.Equals(MergePolicyConstants.StandardMergePolicyName, StringComparison.OrdinalIgnoreCase) ||
4039
policy.Name.Equals(MergePolicyConstants.NoRequestedChangesMergePolicyName, StringComparison.OrdinalIgnoreCase) ||
4140
policy.Name.Equals(MergePolicyConstants.DontAutomergeDowngradesPolicyName, StringComparison.OrdinalIgnoreCase) ||
4241
policy.Name.Equals(MergePolicyConstants.ValidateCoherencyMergePolicyName, StringComparison.OrdinalIgnoreCase))

‎src/Microsoft.DotNet.Darc/Darc/Operations/AddSubscriptionOperation.cs‎

Lines changed: 3 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -46,14 +46,9 @@ public AddSubscriptionOperation(
4646
/// </summary>
4747
public override async Task<int> ExecuteAsync()
4848
{
49-
if (_options.IgnoreChecks.Any() && !_options.AllChecksSuccessfulMergePolicy && !_options.StandardAutoMergePolicies)
49+
if (_options.IgnoreChecks.Any() && !_options.AllChecksSuccessfulMergePolicy)
5050
{
51-
_logger.LogError("--ignore-checks must be combined with --all-checks-passed or --standard-automerge");
52-
return Constants.ErrorCode;
53-
}
54-
if (_options.CodeFlowCheckMergePolicy && !_options.SourceEnabled)
55-
{
56-
_logger.LogError("--code-flow-check can only be used with --source-enabled subscriptions");
51+
Console.WriteLine($"--ignore-checks must be combined with --all-checks-passed");
5752
return Constants.ErrorCode;
5853
}
5954

@@ -96,7 +91,7 @@ public override async Task<int> ExecuteAsync()
9691
new MergePolicy
9792
{
9893
Name = MergePolicyConstants.StandardMergePolicyName,
99-
Properties = new() { [MergePolicyConstants.IgnoreChecksMergePolicyPropertyName] = JToken.FromObject(_options.IgnoreChecks) }
94+
Properties = []
10095
});
10196
}
10297

@@ -109,23 +104,6 @@ public override async Task<int> ExecuteAsync()
109104
});
110105
}
111106

112-
if (_options.CodeFlowCheckMergePolicy)
113-
{
114-
if (_options.StandardAutoMergePolicies)
115-
{
116-
_logger.LogInformation("Code flow check merge policy is already included in standard auto-merge policies. Skipping");
117-
}
118-
else
119-
{
120-
mergePolicies.Add(
121-
new MergePolicy
122-
{
123-
Name = MergePolicyConstants.CodeflowMergePolicyName,
124-
Properties = []
125-
});
126-
}
127-
}
128-
129107
if (_options.Batchable && mergePolicies.Count > 0)
130108
{
131109
Console.WriteLine("Batchable subscriptions cannot be combined with merge policies. " +

0 commit comments

Comments
 (0)