cobra
cobra copied to clipboard
spf13
feat: panic on CheckErr
os.Exit(1) kill process without defering functions. panic can be recover if needed.
Signed-off-by: Guilhem Lettron [email protected]
This seems like a better approach to me as well. People do tend to code/script around these sorts of things so I'm slightly curious if it would be a breaking change for anyone.
If merged and you got a panic instead of the os.Exit, the exit code would ultimately still be non-zero unless manually overwritten by their deferral. Otherwise you'd just get your deferrals (which presumably they wanted). I think it would be a bit weird to explicitly write deferrals you want run but not if cobra reports the panic.
@jpmcb Tiny PR we could merge and get off the books if you're ok with the change. As a new maintainer I'm trying to be cautious about what you consider breaking changes.
This seems like a better approach to me as well. People do tend to code/script around these sorts of things so I'm slightly curious if it would be a breaking change for anyone.
If merged and you got a panic instead of the os.Exit, the exit code would ultimately still be non-zero unless manually overwritten by their deferral. Otherwise you'd just get your deferrals (which presumably they wanted). I think it would be a bit weird to explicitly write deferrals you want run but not if cobra reports the panic.
Although I agree with the benefits, I'm also worried about backwards-compatibility, especially since this function is public and is used by other programs. I would err on the side of caution personally.
I would really love panics rather than os.Exit(1) because it would allow me to add telemetry to errors.
I understand the concern is with backward compatibility. Could the ability to panic be added behind some sort of feature flag instead of being default behavior?
os.Exit is also problematic in tests as it produces close to useless output in terms of debugging. In our case we do not use cobra's CheckErr ever, but rather have our own function (which to be fair is simple enough to do anyway).
