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

feat(kubernetes): Allow to filter context names #4505

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

DanielKneipp
Copy link

Allow filtering of kubernetes context names

Description

This change should allow the user to specify a list of namespace names to be ignored via the variable filter_contexts. Any context that matches a name on this list will be ignored.

Motivation and Context

This is to allow the user to ignore regular local contexts like docker-desktop. Basically, I want to have a way to ignore the default context I have.

How Has This Been Tested?

Tested with cargo test kubernetes

  • I have tested using MacOS

Checklist:

  • I have updated the documentation accordingly.
  • I have updated the tests accordingly.

Copy link
Member

@davidkna davidkna left a comment

Choose a reason for hiding this comment

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

This seems fine, but please rename the option to ignore_contexts to better align with similar options elswhere.

@jankatins jankatins mentioned this pull request Oct 24, 2022
8 tasks
Copy link
Member

@davidkna davidkna left a comment

Choose a reason for hiding this comment

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

LGTM, but we may want to wait how #614 etc. progress before merging.

@DanielKneipp
Copy link
Author

No rush, but is that PR related somehow? I know it would allow conditional formatting based on the context name. I couldn't see a way to just ignore a context.

@davidkna
Copy link
Member

@DanielKneipp I was thinking it might make sense to move the ignore option into display alongside with the style options etc.

@jankatins
Copy link
Contributor

jankatins commented Oct 31, 2022

See #4550 for an updated/combined PR (#614 + #1568) for styling environments -> that PR does not contain the ignore feature (yet?)

@DanielKneipp
Copy link
Author

@davidkna Agreed! this way we can have all context-related config unified. I might have some time to check this out later this weekend. Should I commit on #4550 or is a new PR preferred after this one gets merged? What do you guys think @davidkna @jankatins ?

@jankatins
Copy link
Contributor

jankatins commented Nov 3, 2022

@DanielKneipp I would appreciate a review of #4550 and it's probably best to wait for a comment by one of the starship maintainer on what direction #4550 should take (there was not yet any maintainer review on that PR). :-)

@jankatins
Copy link
Contributor

This PR would need a rebase (and probably a rework :-() now that #4550 is merged. I would also really love to have this. @DanielKneipp

@jankatins
Copy link
Contributor

@DanielKneipp Do you think you still find time (and interest) to finish this PR or would it be ok if I take this over to rebase and rework it (in a new PR with your name as coauthor in the commit).

@DanielKneipp
Copy link
Author

DanielKneipp commented Mar 5, 2024

Oh feel free @jankatins! By the way, I also added some other ignore options to mix and match k8s contexts and namespaces

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.

None yet

3 participants