Skip to content
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

Add Property maximize_feature_capability #3330

Closed
wants to merge 1 commit into from

Conversation

adityagesh
Copy link
Collaborator

maximize_feature_capability can accept coma seperated strings. This is used to enable a feature capability

maximize_feature_capability can accept coma seperated strings.
This is used to enable a feature capability
Copy link
Collaborator

@mcgov mcgov left a comment

Choose a reason for hiding this comment

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

I love this option, though this option might be reduant. Have you tested whether ignore_capability satifies this requirement?

Less important, but I have always hated the nomenclature of 'maximize_capability'. We're overriding LISA's VM feature metadata when resolving whether an environment satisfies a test's requirements. We're not really maximizing any value, it's ignoring things. Maybe it's just me.
Maybe a name like override_capability_metadata or `force_enable_capability' or something that makes it clear that we're actually ignoring something or changing the way we evaluate an operation.
No code comments other than these nit picks, it looks great 👍

@adityagesh
Copy link
Collaborator Author

Thanks for reviewing @mcgov
ignore_capability does not satisfy the requirement. We can use it to run the test case, but the feature will not be enabled.

I agree with your thoughts on the naming, maximize_capability is indeed confusion. I named this similar to maximize_capability since it serve very similar purpose.

However, I agree that we should rename to something like 'force_enable_feature'. I will rename it before merging this patch

@adityagesh adityagesh closed this Jul 12, 2024
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