Skip to content
This repository was archived by the owner on Dec 12, 2023. It is now read-only.

Support for hidden option field#77

Merged
adriantpaez merged 2 commits into
developfrom
feat/hidden-option-field
Sep 18, 2023
Merged

Support for hidden option field#77
adriantpaez merged 2 commits into
developfrom
feat/hidden-option-field

Conversation

@AntiD2ta
Copy link
Copy Markdown
Contributor

Changes:

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Testing

Requires testing Yes

In case you checked yes, did you write tests? Yes

@AntiD2ta AntiD2ta self-assigned this Sep 15, 2023
Copy link
Copy Markdown
Contributor

@adriantpaez adriantpaez left a comment

Choose a reason for hiding this comment

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

I tried the following command

eigenlayer install --profile option-returner --no-prompt --version v5.5.0 https://github.com/NethermindEth/mock-avs-pkg

And returns an error as the following image shows:

image

We need to manage properly the case when the --no-prompt flag is used with hidden options. This is because it tries to use the default value, and hidden properties do not have one.

@AntiD2ta
Copy link
Copy Markdown
Contributor Author

AntiD2ta commented Sep 16, 2023

I tried the following command

eigenlayer install --profile option-returner --no-prompt --version v5.5.0 https://github.com/NethermindEth/mock-avs-pkg

And returns an error as the following image shows:

image

We need to manage properly the case when the --no-prompt flag is used with hidden options. This is because it tries to use the default value, and hidden properties do not have one.

What do you suggest @adriantpaez ? The wizard's behavior is not wrong, the error message is ok. We clearly define in the Spec that profile options with hidden set on true will have their default values ignored.

What I see we can do is to tell the users they have to use the --option.option-name flag to set that option value if they want to use --no-prompt

@adriantpaez
Copy link
Copy Markdown
Contributor

In the future, we will handle the default value management for the hidden option through a new PR, probably with a different error message. This is necessary because we need to prioritize the merging of other pending PRs. It's important to note that the current behavior is not incorrect.

@adriantpaez adriantpaez merged commit f8c93b6 into develop Sep 18, 2023
@adriantpaez adriantpaez deleted the feat/hidden-option-field branch September 18, 2023 12:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants