Skip to content

feat(completion): dynamic completion for suitable flags#702

Merged
rkpattnaik780 merged 2 commits intomainfrom
dynamic_flags
Jun 11, 2021
Merged

feat(completion): dynamic completion for suitable flags#702
rkpattnaik780 merged 2 commits intomainfrom
dynamic_flags

Conversation

@rkpattnaik780
Copy link
Contributor

Add dynamic flag completion for --topic of rhoas kafka consumergroup list and --provider for rhoas kafka create.

Addresses #591

Verification Steps

  1. Press tab twice after rhoas kafka consumergroup list --topic, it should auto-fill according to topics in the instance.
  2. Press tab twice after rhoas kafka create <instance name> --provider, it should suggest the available providers.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation change
  • Other (please specify)

Checklist

  • Documentation added for the feature
  • CI and all relevant tests are passing
  • Code Review completed
  • Verified independently by reviewer

craicoverflow
craicoverflow previously approved these changes Jun 8, 2021
validProviders = []string{}
directive = cobra.ShellCompDirectiveNoSpace

conn, err := f.Connection(connection.DefaultConfigRequireMasAuth)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason that DefaultConfigRequireMasAuth is needed?

@craicoverflow craicoverflow self-requested a review June 8, 2021 08:31
@craicoverflow craicoverflow dismissed their stale review June 8, 2021 08:31

Pending response to question about DefaultConfigRequireMasAuth


api := conn.API()

cloudProviderResponse, _, err := api.Kafka().GetCloudProviders(context.Background()).Execute()
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should start chaining the entire conn like this where possible:

Suggested change
cloudProviderResponse, _, err := api.Kafka().GetCloudProviders(context.Background()).Execute()
cloudProviderResponse, _, err := conn.API().Kafka().GetCloudProviders(context.Background()).Execute()

@rkpattnaik780 rkpattnaik780 merged commit 145d05f into main Jun 11, 2021
@rkpattnaik780 rkpattnaik780 deleted the dynamic_flags branch June 11, 2021 06:57
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