-
Notifications
You must be signed in to change notification settings - Fork 18
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
target history feature #162
Conversation
This PR is Ready for review |
This PR is ready for review, @gardener/gardenctl-v2-maintainers |
Hi Ted,
-> I checked how the binary / history feature behaved and it reminds me of your very first implementation proposal in gardenctl (v1). You only store the command itself but loose the complete context. We already discussed this excessively in the past and you had improved it in the v1 implementation so that the context is not lost. That's why I'm a bit surprised why this implementation is again a step backwards, compared to the v1 implementation. |
you do not only need the complete context for display purposes, but also to restore the target properly |
This PR is ready for review, https://github.com/orgs/gardener/teams/gardenctl-v2-maintainers |
Can we process this PR Thanks? @gardener/gardenctl-v2-maintainers |
Hi @tedteng, somehow it does not write the history in my case When debugging it, it also did not run into gardenctl-v2/pkg/cmd/target/target.go Lines 40 to 46 in 6be635b
|
Hi @petersutter, I'm just back from the vacation. Did the issue still occur? I was try to on my local and one more new Mac not meet this error again. version |
@tedteng sorry for the late reply, yes the issue still occurs. @holgerkoser @grolu does it work for you? |
use aliase name ghh, and use PersistentPostRunE fix alias and history write when target only
…it is an empty string
PR is rebased feel free to process it thanks. @gardener/gardenctl-v2-maintainers |
@tedteng You need rebase this pull request with latest master branch. Please check. |
/close this PR. as it can simply create an alias after
|
What this PR does / why we need it:
target history feature and fuzzy targeting
Which issue(s) this PR fixes:
Fixes #133
Fixes #135
Special notes for your reviewer:
the Fuzzy targeting required
fzf
installedthe target history feature only records target history, which means
target view
, andtarget unset
do not count in the implementPowerShell is not supported yet.
use
gh
as alias name callgardenctl target history
featureRelease note: