Skip to content

Conversation

JohnMcPMS
Copy link
Member

@JohnMcPMS JohnMcPMS commented Feb 5, 2025

Change

In response to some other discussions, update the configuration environment behavior:

  1. Add an environment at the set level
  2. Don't inherit environment data into child units, and don't promote environment data when serializing

Updated the one consumer of environment (dynamic factory) to be responsible for the flow of the security context from the set to the immediate child units.

Also improved serialization of groups to output the non-resource properties.

Microsoft Reviewers: Open in CodeFlow

@JohnMcPMS JohnMcPMS requested a review from a team as a code owner February 5, 2025 17:42
yao-msft
yao-msft previously approved these changes Feb 5, 2025
/// </summary>
/// <param name="set">The set to output.</param>
/// <returns>The string.</returns>
public static string ToYaml(this ValueSet set)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw this is only used in tests. Is it expected to be used in product code in the future? If not, shall we move the code to test common?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I honestly didn't notice that this was in the processor; I just assumed that it was in the unit tests as that is how I navigated there (via F12 from a test).

@@ -432,6 +432,11 @@ namespace Microsoft.Management.Configuration

[contract(Microsoft.Management.Configuration.Contract, 3)]
{
// The environment in which to process the configuration set.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is the configuration contract version defined? Is it tied to the schema version? (i.e. not tied to our releases since it's still in preview only?)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have a strict contract number mapping to anything outside of the interface definition.

@JohnMcPMS JohnMcPMS merged commit 71e3a17 into microsoft:master Feb 6, 2025
9 checks passed
@JohnMcPMS JohnMcPMS deleted the config-env branch February 6, 2025 01:45
JohnMcPMS added a commit to JohnMcPMS/winget-cli that referenced this pull request Feb 6, 2025
## Change
In response to some other discussions, update the configuration
environment behavior:
1. Add an environment at the set level
2. Don't inherit environment data into child units, and don't promote
environment data when serializing

Updated the one consumer of environment (dynamic factory) to be
responsible for the flow of the security context from the set to the
immediate child units.

Also improved serialization of groups to output the non-resource
properties.
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.

2 participants