Improve darc update-subscription by allowing to specify policy checks
#4680
Conversation
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
…hecks in standard automerge policies
dkurepa
changed the title
darc update-subscription by allowing to specify policy checks
premun
reviewed
Apr 15, 2025
src/Microsoft.DotNet.Darc/Darc/Options/UpdateSubscriptionCommandLineOptions.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.DotNet.Darc/Darc/Operations/UpdateSubscriptionOperation.cs
Outdated
Show resolved
Hide resolved
premun
reviewed
Apr 15, 2025
| .ToObject<IEnumerable<string>>() | ||
| ?? []; | ||
|
|
||
| private void AddMergePolicyWithIgnoreChecksIfMissing(List<MergePolicy> mergePolicies, string policy) |
There was a problem hiding this comment.
This method name feels a bit too descriptive. Maybe AddMergePolicyWithDefaults?
Also, what is trying to do exactly? It seems its trying to append ignore checks from a specific policy if it exists and if not then add a default policy which seems a bit too different of operations to combine into one function, no?
| }); | ||
| } | ||
|
|
||
| if (_options.SourceFlowCheckMergePolicy) |
There was a problem hiding this comment.
This snippet is quite similar to the one in AddSubscriptionOperation and seems to be tied to source flow rather than the operations themselves. Should it be maybe in some helper object instead?
premun
reviewed
Apr 15, 2025
src/Microsoft.DotNet.Darc/Darc/Models/PopUps/MergePoliciesPopUpHelpers.cs
Outdated
Show resolved
Hide resolved
…pHelpers.cs Co-authored-by: Přemek Vysoký <premek.vysoky@microsoft.com>
premun
approved these changes
Apr 15, 2025
premun
added a commit
to premun/arcade-services
that referenced
this pull request
Apr 16, 2025
…icy checks (dotnet#4680)" This reverts commit 15c4f8e.
premun
added a commit
to dkurepa/arcade-services
that referenced
this pull request
Apr 17, 2025
…ks (dotnet#4680) <!-- 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#4612 dotnet#2599 - Allows subscription merge policy updates from the `update-subscription command - Allows adding explicitly adding coherency check policies - Allows specifying `ignore-checks` for the standard automerge policy --------- Co-authored-by: Přemek Vysoký <premek.vysoky@microsoft.com>
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.
#4612
#2599
ignore-checksfor the standard automerge policy