Skip to content

Config command support#798

Merged
wtrocki merged 11 commits intomainfrom
config
Jul 6, 2021
Merged

Config command support#798
wtrocki merged 11 commits intomainfrom
config

Conversation

@wtrocki
Copy link
Collaborator

@wtrocki wtrocki commented Jul 6, 2021

Verification

[wtrocki@graphapi app-services-cli (config)]$ rhoas config devPreview true
Developer Preview commands activated. Use help command to view them.
[wtrocki@graphapi app-services-cli (config)]$ rhoas config devPreview trueds
Error: invalid argument "trueds" for "rhoas config devPreview"
[wtrocki@graphapi app-services-cli (config)]$ rhoas config devPreview false 
Developer Preview commands deactivated.
[wtrocki@graphapi app-services-cli (config)]$ 

Approach

In 0b79620 I have used single command approach and 2 arguments - one which is config key second is value.
I think that is overly complex and convoluted.
Current approach uses separate command for each key.

Drawback is for that approach we getting documentation file generated and there is little bit of duplication in docs, but much simpler and easier to work with.

What is outstanding

  • base implementation
  • unit tests
  • Error handling mapping instead of system error
  • I18n :)

@wtrocki wtrocki changed the title Config Config command support Jul 6, 2021
@wtrocki wtrocki requested a review from craicoverflow July 6, 2021 14:53
@wtrocki wtrocki force-pushed the config branch 3 times, most recently from f34894f to 6576f52 Compare July 6, 2021 15:46
@wtrocki
Copy link
Collaborator Author

wtrocki commented Jul 6, 2021

@craicoverflow that will be quick review (small command/refactoring here)

Copy link
Contributor

@craicoverflow craicoverflow left a comment

Choose a reason for hiding this comment

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

I suggest changing the command to dev-preview - all good otherwise

Co-authored-by: Enda <ephelan@redhat.com>
@wtrocki wtrocki force-pushed the config branch 2 times, most recently from a4e5dc6 to b64ea9e Compare July 6, 2021 16:01
@wtrocki wtrocki merged commit c33185d into main Jul 6, 2021
@wtrocki wtrocki deleted the config branch July 6, 2021 16:20
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