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(kubens): added force flag to switch namespaces even if it doesn'… #416

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

Conversation

idsulik
Copy link

@idsulik idsulik commented Jan 21, 2024

Hello!
Added --force && -f flags to kubens command to switch namespace even if it doesn't exist.
issue: #333

usage example

$ kubens not-found-namespace --force
Context "test" set.
Active namespace is "not-found-namespace".
---
$ kubens not-found-namespace --f
Context "test" set.
Active namespace is "not-found-namespace".

@idsulik
Copy link
Author

idsulik commented Apr 13, 2024

@ahmetb Hi!
Should I do anything else or I just need to wait?

@ahmetb
Copy link
Owner

ahmetb commented Apr 15, 2024

Sorry, I mostly haven't gotten around reviewing it

cmd/kubens/help.go Outdated Show resolved Hide resolved
@idsulik
Copy link
Author

idsulik commented Jun 24, 2024

@ahmetb hi! kindly remind you

Copy link
Owner

@ahmetb ahmetb left a comment

Choose a reason for hiding this comment

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

Sorry for late review.

README.md Outdated
Context "test" set.
Active namespace is "not-found-namespace".
---
$ kubens not-found-namespace --f
Copy link
Owner

Choose a reason for hiding this comment

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

Double dash is weird to see for single letter flags. Mistake?

Copy link
Author

Choose a reason for hiding this comment

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

yes, it's mistake, pushed fix

force := false

if n == 2 {
force = argv[1] == "--force" || argv[1] == "-f"
Copy link
Owner

Choose a reason for hiding this comment

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

I think slices.Contains is better here? IMO people should be able to do both:

  • kubens foo -f
  • kubens -f foo

Copy link
Author

Choose a reason for hiding this comment

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

pushed changes to support both versions


if n == 2 {
force = argv[1] == "--force" || argv[1] == "-f"
}
Copy link
Owner

Choose a reason for hiding this comment

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

There are some tests of this file, do you mind editing?

Copy link
Author

Choose a reason for hiding this comment

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

done

@idsulik
Copy link
Author

idsulik commented Jun 30, 2024

@ahmetb hi! is there anything else to do within the PR?)

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

2 participants