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

fix panic with some missing values for params or help #188

Closed
wants to merge 1 commit into from

Conversation

czhujer
Copy link
Contributor

@czhujer czhujer commented May 15, 2023

Fix for: #187

@czhujer
Copy link
Contributor Author

czhujer commented May 15, 2023

FYI @nerdeveloper :)

@czhujer
Copy link
Contributor Author

czhujer commented May 17, 2023

did you check the fix @nerdeveloper ? Or is there some protocol/process which i have to follow? :)

@czhujer
Copy link
Contributor Author

czhujer commented May 20, 2023

Hi @technosophos and @nerdeveloper,

Is it possible to merge this an create a new tag for a release?

I would like to use official version of this plugin, not my fork..

Thanks,
P.

@nerdeveloper
Copy link
Member

@scbizu Please take a look

@nerdeveloper nerdeveloper requested a review from scbizu May 22, 2023 22:58
@scbizu
Copy link
Contributor

scbizu commented May 23, 2023

I wonder why we can't make the -h / --help works as expect rather than disable it ? 🤔

@nerdeveloper
Copy link
Member

Exactly, I have been trying to debug this but can’t seem to get anything working ATM

@czhujer
Copy link
Contributor Author

czhujer commented May 24, 2023

help param is ommited, because it's catched by helm binary itself (it's used same in several other helm plugins).
It's wierd, looks like i'm experiencing different behavior then rest of the poeple :)

@czhujer
Copy link
Contributor Author

czhujer commented May 24, 2023

if I remove ommiting help from args array (L131:135), i see this error:

$ bin/darwin/amd64/helm-cm-push -h
pflag: help requested
$ helm cm-push --help
pflag: help requested

@czhujer
Copy link
Contributor Author

czhujer commented May 24, 2023

btw can you take a look first on this please? #190

@scbizu
Copy link
Contributor

scbizu commented May 25, 2023

help param is ommited, because it's catched by helm binary itself (it's used same in several other helm plugins). It's wierd, looks like i'm experiencing different behavior then rest of the poeple :)

It's quite weird , I will take a deep look this weekend , thank you ~

@czhujer
Copy link
Contributor Author

czhujer commented May 25, 2023

help param is ommited, because it's catched by helm binary itself (it's used same in several other helm plugins). It's wierd, looks like i'm experiencing different behavior then rest of the poeple :)

It's quite weird , I will take a deep look this weekend , thank you ~

cool.. thank you :)

@scbizu
Copy link
Contributor

scbizu commented May 27, 2023

hey @czhujer , I create another PR(#191) to fix this , you can go to test if your issue is fixed.

@czhujer
Copy link
Contributor Author

czhujer commented May 30, 2023

Hi @scbizu , your PR looks good to me :)

@scbizu
Copy link
Contributor

scbizu commented May 30, 2023

Ok , I am gonna close this PR , please watch my PR , i will merge it ASAP

@scbizu scbizu closed this May 30, 2023
@czhujer
Copy link
Contributor Author

czhujer commented May 30, 2023

yes, thank you for your time :)

@czhujer
Copy link
Contributor Author

czhujer commented May 31, 2023

and can you check also this please #194 ?
(for sync with our codebase)

@scbizu
Copy link
Contributor

scbizu commented Jun 1, 2023

Merged :)

@czhujer
Copy link
Contributor Author

czhujer commented Jun 1, 2023

Awesome 🔥🔥🔥

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.

3 participants