-
Notifications
You must be signed in to change notification settings - Fork 14
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
feat: able to remove an environment #167
feat: able to remove an environment #167
Conversation
Signed-off-by: blu3beri <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice improvement. Did not test locally, approving to unblock but please check out the copy updates.
@@ -105,6 +107,8 @@ impl HelpStrings { | |||
|
|||
HelpStrings::Configuration => "Add agents to your configuration or view your current configuration. To quickly get started run the following command: aries-cli configuration add --default", | |||
HelpStrings::ConfigurationAdd => "Add a new, or overwrite an existing, agent your configuration file", | |||
HelpStrings::ConfigurationRemove => "PERMANENTLY remove an agent from your configuration", | |||
HelpStrings::ConfigurationRemoveEnvironment => "Environment to delete", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consistency: let's pick either "remove" or "delete". Given that the command is remove let's go with that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah sounds good!
cli/src/modules/configuration.rs
Outdated
debug!( | ||
"{} environment {} from the configuration", | ||
"Deleting".bold().red(), | ||
environment.yellow() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should only reach for yellow when warning about something. I think just bold works here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah seems okay for me.
Signed-off-by: blu3beri <[email protected]>
2fe666c
to
eab29b9
Compare
Signed-off-by: blu3beri <[email protected]>
Closes #165
(@karimStekelenburg I also assigned you if you want to check it out)
Signed-off-by: blu3beri [email protected]