chartmuseum / helm-push

Helm plugin to push chart package to ChartMuseum
Apache License 2.0
670 stars 170 forks source link

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

Closed czhujer closed 1 year ago

czhujer commented 1 year ago

Fix for: https://github.com/chartmuseum/helm-push/issues/187

czhujer commented 1 year ago

FYI @nerdeveloper :)

czhujer commented 1 year ago

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

czhujer commented 1 year ago

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 commented 1 year ago

@scbizu Please take a look

scbizu commented 1 year ago

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

nerdeveloper commented 1 year ago

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

czhujer commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

btw can you take a look first on this please? https://github.com/chartmuseum/helm-push/pull/190

scbizu commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

czhujer commented 1 year ago

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

scbizu commented 1 year ago

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

czhujer commented 1 year ago

yes, thank you for your time :)

czhujer commented 1 year ago

and can you check also this please https://github.com/chartmuseum/helm-push/pull/194 ? (for sync with our codebase)

scbizu commented 1 year ago

Merged :)

czhujer commented 1 year ago

Awesome 🔥🔥🔥