-
Notifications
You must be signed in to change notification settings - Fork 80
Support --subscription
and --excluded-assets
arguments in darc vmr codeflow operations
#5389
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: premun <[email protected]>
Co-authored-by: premun <[email protected]>
--subscription
argument in darc vmr codeflow operations
src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/CodeFlowOperation.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/CodeFlowOperation.cs
Outdated
Show resolved
Hide resolved
…xcludedAssets as method argument Co-authored-by: premun <[email protected]>
There was a problem hiding this 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 adds support for the --subscription
argument to VMR codeflow operations (darc vmr forwardflow
and darc vmr backflow
), enabling users to simulate subscription behavior during manual codeflow operations. This enhances debugging capabilities and provides consistency with existing darc update-dependencies --subscription
functionality.
Key changes include:
- Added
--subscription
command-line option to codeflow operations - Implemented subscription metadata fetching with automatic build selection and excluded assets extraction
- Replaced hardcoded empty excluded assets arrays with subscription-derived values
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
CodeFlowCommandLineOptions.cs | Added SubscriptionId property and command-line option definition |
CodeFlowOperation.cs | Implemented subscription metadata fetching, validation, and option population logic |
ForwardFlowOperation.cs | Updated to accept and use excluded assets parameter from subscription |
BackflowOperation.cs | Updated to accept and use excluded assets parameter from subscription |
src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/CodeFlowOperation.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/CodeFlowOperation.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <[email protected]>
/// Fetch subscription metadata and populate command options based on subscription settings. | ||
/// This allows the subscription to be simulated using the existing codeflow logic. | ||
/// </summary> | ||
private async Task<IReadOnlyList<string>> PopulateOptionsFromSubscriptionAsync() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like that nothing about this method name tells you it's gonna return a list of excluded assets. Think it'd be better if it was a parameter, we passed a list, and this method would fill it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. But I think you can't have out
parameter in an async method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot can you add a new optional argument to the code flow commands --excluded-assets
which would work similarly to the others (like build
) that it would be taken from a subscription first and overriden by the argument, if the argument is set.
We then could store it in the _options
and would not need to pass it around.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Added --excluded-assets
option that works like --build
- it's taken from the subscription first and can be overridden by the command-line argument. Now stored in _options.ExcludedAssets
and parsed when needed, eliminating the need to pass it around as a method parameter.
…nstead of passing as parameter Co-authored-by: premun <[email protected]>
--subscription
argument in darc vmr codeflow operations--subscription
and --excluded-assets
arguments in darc vmr codeflow operations
Summary
This PR adds support for the
--subscription
and--excluded-assets
arguments todarc vmr forwardflow
anddarc vmr backflow
operations, allowing users to simulate subscription behavior for VMR code flow operations. This brings consistency with the existingdarc update-dependencies --subscription
functionality.Problem
Previously, when running VMR codeflow operations, there was no way to simulate how a subscription would behave. Additionally, excluded assets from subscriptions were not being utilized (left as empty arrays), as indicated by TODO comments in the code referencing issue #5313.
Solution
Added subscription support to the codeflow operations by:
--subscription
option toCodeFlowCommandLineOptions
(shared by both forwardflow and backflow)--excluded-assets
option toCodeFlowCommandLineOptions
that can be used standalone or to override subscription's excluded assetsCodeFlowOperation
base class that:_options.ExcludedAssets
(can be overridden by command-line argument)--build
is provided)--build
--subscription
with--ref
is prevented;--build
and--excluded-assets
can be used with--subscription
)Usage
When
--subscription
is provided, the tool automatically:--build
is specified)--excluded-assets
is specified)Benefits
--subscription
with--build
to use a specific build with the subscription's excluded assets--subscription
with--excluded-assets
to override subscription settings, or use--excluded-assets
independentlydarc update-dependencies --subscription
, where options can be overridden via command-line argumentsChanges
Fixes #5313
Original prompt
Fixes #5388
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.