Skip to content

Added Conditional parameter sets feature #527

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

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

RakeshwarK
Copy link
Contributor

No description provided.

/// Provides a JSON converter that can handle the serialization/deserialization of
/// <see cref="List{T}"/> objects where T is <see cref="IDictionary{TKey, TValue}"/> with string keys and <see cref="IConvertible"/> values.
/// </summary>
public class ParameterDictionaryListJsonConverter : JsonConverter
Copy link
Contributor

Choose a reason for hiding this comment

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

Naming: ParameterDictionaryCollectionJsonConverter

@@ -188,6 +210,35 @@ public class ProfileExpressionEvaluator : IExpressionEvaluator
Outcome = evaluatedExpression
});
}),
// Expression: {Architecture}
Copy link
Contributor

Choose a reason for hiding this comment

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

Naming: CpuArchitecture.

This is the general name we use throughout the rest of our codebase.

@@ -825,9 +826,113 @@ private void InitializeProfile(ExecutionProfile profile)

ValidationResult result = ExecutionProfileValidation.Instance.Validate(profile);
result.ThrowIfInvalid();

// Process conditional parameters (ParametersOn feature)
await this.ProcessParametersOnAsync(profile, dependencies);
Copy link
Contributor

Choose a reason for hiding this comment

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

Similarly to how there is an extension method ExecutionProfile.Inline(), the logic to determine which parameters should be used from the conditional set should be an extension method as well. You may want to just extend the Inline() method to take in the IServiceCollection.

Recommendation:
I know that there are dependencies in IServiceCollection that cannot be referenced from the VirtualClient.Contracts project. We can likely just move the extensions method class to the VirtualClient.Core project to address the scenario.

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