Promote source priority to a feature - #6509
Conversation
|
/azp run |
|
Azure Pipelines: 1 pipeline(s) were filtered out due to trigger conditions. |
There was a problem hiding this comment.
🟢 Approval recommended
The gating removal is consistent and well covered; only a non-blocking documentation clarification remains.
Pull request overview
Promotes source priority from experimental gating to a standard CLI and DSC feature.
Changes:
- Removes the experimental setting and feature checks.
- Enables priority ordering, selection, output, and DSC operations by default.
- Updates tests, documentation, and release notes.
File summaries
| File | Description |
|---|---|
src/PowerShell/tests/Microsoft.WinGet.DSC.Tests.ps1 |
Removes DSC feature setup. |
src/PowerShell/tests/Microsoft.WinGet.Client.Tests.ps1 |
Removes PowerShell feature setup and restoration. |
src/AppInstallerRepositoryCore/SourceList.cpp |
Always sorts sources by priority. |
src/AppInstallerCommonCore/UserSettings.cpp |
Removes priority-setting validation. |
src/AppInstallerCommonCore/Public/winget/UserSettings.h |
Retires the priority setting mapping. |
src/AppInstallerCommonCore/Public/winget/ExperimentalFeature.h |
Removes the experimental feature enum. |
src/AppInstallerCommonCore/ExperimentalFeature.cpp |
Removes feature lookup and configuration. |
src/AppInstallerCLITests/Sources.cpp |
Tests ordering regardless of policy. |
src/AppInstallerCLITests/SourceFlow.cpp |
Adds CLI, selection, output, and DSC coverage. |
src/AppInstallerCLIE2ETests/SourceCommand.cs |
Removes source test feature setup. |
src/AppInstallerCLIE2ETests/README.md |
Updates enabled-feature example. |
src/AppInstallerCLIE2ETests/InstallCommand.cs |
Removes install test feature setup. |
src/AppInstallerCLIE2ETests/Helpers/WinGetSettingsHelper.cs |
Stops configuring the retired setting. |
src/AppInstallerCLIE2ETests/DSCv3SourceResourceCommand.cs |
Removes DSCv3 feature setup. |
src/AppInstallerCLICore/Workflows/WorkflowBase.cpp |
Always applies priority filtering. |
src/AppInstallerCLICore/Workflows/SourceFlow.cpp |
Always displays and exports priority. |
src/AppInstallerCLICore/Commands/DscSourceResource.cpp |
Removes DSC experimental checks. |
src/AppInstallerCLICore/Argument.cpp |
Makes the priority argument generally available. |
schemas/JSON/settings/settings.schema.0.2.json |
Removes the retired setting. |
doc/windows/package-manager/winget/source.md |
Documents editing and source priority. |
doc/Settings.md |
Removes experimental priority documentation. |
doc/ReleaseNotes.md |
Announces feature promotion. |
Review details
- Files reviewed: 22/22 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
JohnMcPMS
left a comment
There was a problem hiding this comment.
The biggest issue with this PR is that the feature branch name seems to be based solely on this change and not the overall feature.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
71b0c59
into
feature/multi-source-deduplication
ranm-msft
left a comment
There was a problem hiding this comment.
Read through the de-gating at ffc7ab8. The mechanical removal looks complete and the new native tests are the good kind: SourcePriority_SearchResult covers the tied ({0,2,2}), all-default ({0,0,0}) and all-negative ({-3,-1,-2}) cases, not just the happy path, and running each with ExperimentalFeatures policy set to both NotConfigured and Disabled is exactly the right way to prove the feature no longer rides on that policy. Compatibility for anyone who had experimentalFeatures.sourcePriority set also looks clean — the Experimental definition in settings.schema.0.2.json has no additionalProperties: false, and UserSettings only validates over the known Setting enum, so the stale key is silently ignored rather than rejected.
One gap that this PR seems like the right moment to surface, since it's the "promote to a feature" change:
Group Policy sources parse Priority but never apply it. SourceFromPolicy carries std::optional<int32_t> Priority, GroupPolicy.cpp reads it out of the policy JSON (~L239-242) and round-trips it in ToJsonString (~L371-373). But in SourceList.cpp, the SourceOrigin::GroupPolicy branch builds SourceDetailsInternal by copying Name/Type/Arg/Data/Identifier/Explicit/TrustLevel/pinning and never copies Priority, so it stays default-0.
That was inert while the feature was gated. Now that the stable_sort and the highest-priority narrowing in WorkflowBase.cpp are unconditional, an admin can set Priority in AdditionalSources, have it accepted and echoed back, and still be outranked by any user-added source created with --priority 1.
Intentional for now, or worth a follow-up to copy it through?
📖 Description
Promotes source priority to a regular feature by removing experimental gating from CLI and DSC workflows and retiring the
experimentalFeatures.sourcePrioritysetting.Uses the existing commands and options. Default priorities, selection rules, and tie handling are unchanged. Updates documentation, release notes, and regression coverage.
REST filtering, interactive resolution, and heuristic deduplication remain out of scope for this PR.
🔗 References
🔍 Validation
source add --help,source edit --help,features, anddscv3 source --schema.✅ Checklist
📋 Issue Type
Microsoft Reviewers: Open in CodeFlow