-
Notifications
You must be signed in to change notification settings - Fork 79
Extend exclude assets feature to non-codeflow subscriptions #4984
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
base: main
Are you sure you want to change the base?
Conversation
- Updated popup help text to indicate excluded assets work for all subscriptions - Modified PullRequestUpdater.GetRequiredUpdates to fetch subscription and filter excluded assets - Updated documentation comment on Subscription.ExcludedAssets property - Added helper methods GetExcludedAssetsMatcher and IsExcludedAsset to PullRequestUpdater - Verified existing tests pass to ensure no regression in functionality Co-authored-by: premun <7013027+premun@users.noreply.github.com>
Co-authored-by: premun <7013027+premun@users.noreply.github.com>
src/Microsoft.DotNet.Darc/Darc/Models/PopUps/AddSubscriptionPopUp.cs
Outdated
Show resolved
Hide resolved
…pups Co-authored-by: premun <7013027+premun@users.noreply.github.com>
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.
Please add tests in the existing ProductConstructionService.DependencyFlow.Tests
project for the PullRequestUpdater
that would cover updates where either some or even all packages are excluded.
Co-authored-by: premun <7013027+premun@users.noreply.github.com>
Added comprehensive tests in the existing |
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.
Seems like there are still some ties between excluded assets being only utilized for source-enabled subscriptions. For example in UxHelpers.cs
.
Can you search all usages of ExcludedAssets
and whether we can decouple so that it is valid in non-source-enabled subscriptions?
… subscriptions Co-authored-by: premun <7013027+premun@users.noreply.github.com>
Found and removed all remaining restrictions that tied excluded assets to source-enabled subscriptions. Removed validation checks in SubscriptionPopUp.cs, AddSubscriptionOperation.cs, and UpdateSubscriptionOperation.cs, and updated UxHelpers.cs to display excluded assets for all subscriptions. Changes committed in a9682a4. |
Co-authored-by: premun <7013027+premun@users.noreply.github.com>
src/Microsoft.DotNet.Darc/DarcLib/Models/Darc/AssetFilterExtensions.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VersionFileCodeFlowUpdater.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: dkurepa <91743470+dkurepa@users.noreply.github.com>
src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestUpdater.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: dkurepa <91743470+dkurepa@users.noreply.github.com>
This PR extends the excluded assets feature to work with non-codeflow (regular dependency-only) subscriptions, addressing the requirement that repos should be able to exclude specific packages like arcade while still receiving other dependency updates.
Changes Made
Core Implementation:
PullRequestUpdater.GetRequiredUpdates()
to fetch subscription data and filter out excluded assets before processing dependenciesGetExcludedAssetsMatcher()
andIsExcludedAsset()
that use the same filtering pattern as the existing VMR codeflow implementationbuild.Assets
before callingGetRequiredNonCoherencyUpdates()
Documentation and UI:
AddSubscriptionPopUp.cs
andUpdateSubscriptionPopUp.cs
to remove the "source-enabled subscriptions (VMR code flow)" restrictionSubscription.ExcludedAssets
to indicate it works for all dependency updatesHow It Works
When a subscription update is triggered:
GetRequiredUpdates()
fetches the subscription from the database (including ExcludedAssets)Matcher
from the excluded asset patterns usingGetExcludedAssetsMatcher()
IsExcludedAsset()
before processing dependenciesExample Use Case
A non-VMR repo can now create a subscription that takes the latest VMR build but excludes arcade packages:
This allows the repo to receive all other dependency updates while staying on .NET N-1 arcade.
Testing
Fixes #4806.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.