-
Notifications
You must be signed in to change notification settings - Fork 5
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
Replace process:exit in functions with Result + error handling #58
Conversation
Hey Nico! Super hectic start but hoping it will settle down soon 😓! |
@@ -68,22 +71,32 @@ enum Mode { | |||
} | |||
|
|||
impl Mode { | |||
fn invoke(&self) { | |||
fn invoke(&self) -> Result <(), Error> { | |||
let args = Cli::parse(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
|
||
use crate::{commands::{self}, config, Cli, DEST, KUBECONFIG}; | ||
|
||
fn selection(value: Option<String>, callback: fn() -> String) -> String { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanking you for resolving this ugly hack 👍
This is such a good point! Thank you for pointing it out! |
Keeping my fingers crossed for you! It's been quite hectic over here as well 😰 |
Should I try and fix the issue with the CI before merging? I don't think it's related to my changes specifically |
I don't think so either, Il create a issue to make forks not need the phony secret from this repo (I'm guessing that it's the issue) Merge away! |
Description
Hi Rami, I hope you've had a good start into 2024 :) I'm back with another PR on error handling.
std::process::exit
when encountering an error, which can cause problemspub fn selectable_list(input: Vec<String>) -> String
does not tell you if it can cause the program to terminate, which can be very surprising to developers and requires you to always look at the implementation of a function.I'm looking forward to your feedback :)
Fixes # -
Type of change
src/error.rs
module for error typessrc/modes.rs
now returnResult<(), Error>
(orOption<T>
where appropriate)Changes from a user perspective:
Only error messages have changed.
No item selected
error: no item selected when prompted to select context
error: no item selected when prompted to select namespace
kc foo
)error: no context exists with the name: foo
error: failed to set context: no context exists with the name foo
How Has This Been Tested?
Test Configuration:
Checklist: