Skip to content

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

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Jun 19, 2025

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:

  • Modified PullRequestUpdater.GetRequiredUpdates() to fetch subscription data and filter out excluded assets before processing dependencies
  • Added helper methods GetExcludedAssetsMatcher() and IsExcludedAsset() that use the same filtering pattern as the existing VMR codeflow implementation
  • Assets matching excluded patterns are now filtered from build.Assets before calling GetRequiredNonCoherencyUpdates()

Documentation and UI:

  • Updated popup help text in AddSubscriptionPopUp.cs and UpdateSubscriptionPopUp.cs to remove the "source-enabled subscriptions (VMR code flow)" restriction
  • Updated property documentation on Subscription.ExcludedAssets to indicate it works for all dependency updates

How It Works

When a subscription update is triggered:

  1. GetRequiredUpdates() fetches the subscription from the database (including ExcludedAssets)
  2. Creates a Matcher from the excluded asset patterns using GetExcludedAssetsMatcher()
  3. Filters the build assets using IsExcludedAsset() before processing dependencies
  4. The filtered assets are passed to dependency resolution, effectively excluding them from updates

Example Use Case

A non-VMR repo can now create a subscription that takes the latest VMR build but excludes arcade packages:

Excluded assets: Microsoft.DotNet.Arcade.*

This allows the repo to receive all other dependency updates while staying on .NET N-1 arcade.

Testing

  • Verified existing UpdateAssetsTests pass (10 tests passing) to ensure no regression
  • Confirmed DARC CLI and DependencyFlow components build successfully
  • Implementation follows the same filtering pattern as existing VMR codeflow excluded assets feature

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.

Copilot AI and others added 2 commits June 19, 2025 11:12
- 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>
@Copilot Copilot AI changed the title [WIP] Extend exclude assets feature to non-codeflow subscriptions Extend exclude assets feature to non-codeflow subscriptions Jun 19, 2025
@Copilot Copilot AI requested a review from premun June 19, 2025 11:14
Copilot finished work on behalf of premun June 19, 2025 11:14
…pups

Co-authored-by: premun <7013027+premun@users.noreply.github.com>
Copy link
Member

@premun premun left a 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.

@Copilot Copilot AI requested a review from premun June 19, 2025 11:33
Copilot finished work on behalf of premun June 19, 2025 11:33
Co-authored-by: premun <7013027+premun@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jun 19, 2025

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.

Added comprehensive tests in the existing ProductConstructionService.DependencyFlow.Tests project for the PullRequestUpdater that cover updates where either some or all packages are excluded. The tests verify that excluded assets are properly filtered out before being passed to dependency resolution. Changes committed in 0f73782.

Copilot finished work on behalf of premun June 19, 2025 11:45
Copy link
Member

@premun premun left a 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>
Copy link
Contributor Author

Copilot AI commented Jun 19, 2025

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 w...

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.

@Copilot Copilot AI requested a review from premun June 19, 2025 12:44
Copilot finished work on behalf of premun June 19, 2025 12:44
Co-authored-by: premun <7013027+premun@users.noreply.github.com>
@Copilot Copilot AI requested a review from premun June 19, 2025 12:56
Copilot finished work on behalf of premun June 19, 2025 12:56
@premun premun marked this pull request as ready for review June 19, 2025 13:08
@premun premun requested review from adamzip and dkurepa June 19, 2025 13:08
Co-authored-by: dkurepa <91743470+dkurepa@users.noreply.github.com>
@Copilot Copilot AI requested a review from dkurepa June 20, 2025 14:14
Copilot finished work on behalf of dkurepa June 20, 2025 14:14
Co-authored-by: dkurepa <91743470+dkurepa@users.noreply.github.com>
Copilot finished work on behalf of dkurepa June 20, 2025 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extend exclude assets feature to non-codeflow subscriptions
3 participants