Conversation
Co-authored-by: premun <[email protected]>
Co-authored-by: premun <[email protected]>
--subscription argument in darc vmr codeflow operations
…xcludedAssets as method argument Co-authored-by: premun <[email protected]>
There was a problem hiding this comment.
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
--subscriptioncommand-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 |
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.
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.
I agree. But I think you can't have out parameter in an async method
There was a problem hiding this comment.
@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.
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
--subscriptionand--excluded-assetsarguments todarc vmr forwardflowanddarc vmr backflowoperations, allowing users to simulate subscription behavior for VMR code flow operations. This brings consistency with the existingdarc update-dependencies --subscriptionfunctionality.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:
--subscriptionoption toCodeFlowCommandLineOptions(shared by both forwardflow and backflow)--excluded-assetsoption toCodeFlowCommandLineOptionsthat can be used standalone or to override subscription's excluded assetsCodeFlowOperationbase class that:_options.ExcludedAssets(can be overridden by command-line argument)--buildis provided)--build--subscriptionwith--refis prevented;--buildand--excluded-assetscan be used with--subscription)Usage
When
--subscriptionis provided, the tool automatically:--buildis specified)--excluded-assetsis specified)Benefits
--subscriptionwith--buildto use a specific build with the subscription's excluded assets--subscriptionwith--excluded-assetsto override subscription settings, or use--excluded-assetsindependentlydarc 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.