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 --context flag #215

Merged
merged 5 commits into from
Nov 28, 2024
Merged

Add --context flag #215

merged 5 commits into from
Nov 28, 2024

Conversation

cykl
Copy link
Contributor

@cykl cykl commented Sep 5, 2024

Description

This patch adds an optional --context global flag.
Setting --context overrides the current context to use for this command.
Its behavior is similar to kubectl --context.

Fixes #214

Type of change

  • New feature (non-breaking change which adds functionality)

Documentation

  • the change is mentioned in the ## [Unreleased] section of CHANGELOG.md
  • the configuration yaml was changed and the example config in README.adoc was updated
  • a usage example was added to README.adoc
  • tests for the changes have been implemented (see: Testing your changes)

@cykl
Copy link
Contributor Author

cykl commented Sep 5, 2024

@d-rk I'm learning go on the fly, consider everything I do to be foolish.

I'm not happy that I didn't figure out how to unit test my changes.
Any help is welcome.

I did manually test my changes which seems OK.
Setting --context indeed overrides the default (current) context.
Auto-completion seems OK too

./kafkactl  __complete --context  ""
aws-dev
aws-preprod
aws-prod
aws-staging
:0
Completion ended with directive: ShellCompDirectiveDefault

I'm not sure if it worth mentioning in the README. Any opinion?

@drawks
Copy link

drawks commented Oct 7, 2024

This is great, I was going to create an issue and PR for this exact thing, any chance we can get a code review/merge here? @d-rk

@d-rk
Copy link
Collaborator

d-rk commented Nov 28, 2024

@cykl sorry for the long wait. I add a small refactoring and some tests.
Testing is sadly not always that intuitive in this repo. Perhaps I'll find the time to improve on this sometime.
Anyway, thank you for your contribution 👍

@d-rk d-rk merged commit 0701ca8 into deviceinsight:main Nov 28, 2024
6 checks passed
@cykl
Copy link
Contributor Author

cykl commented Nov 28, 2024

Thanks @d-rk! No worries, we all have busy lives.

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.

Add --context flag
3 participants