Skip to content

reduce pattern matching depth in main hook #278

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

Closed

Conversation

atalii
Copy link
Contributor

@atalii atalii commented May 7, 2023

This is a minor style change and not all that important, but I found it to be somewhat more clear as I was reading through the code. Sudo-rs is a very intriguing project, and I'm excited to watch it reach feature-completion!

@rnijveld
Copy link
Collaborator

rnijveld commented May 8, 2023

Thanks for your interest and for the effort in writing a patch, but I think we are going to have to say no for now. Mostly because this code is still very much in flux and will change in the very near future. I'd prefer to keep the churn to a minimal up to that point. The way we currently display errors that end up in main is not all that useful to end users, and as a result this code will very likely change.

As an aside, I think the original variant is a little bit better in preventing having to call status.code() twice and not needing to use unwrap(), but I agree that your variant has a slightly better structure. The best thing of course would be that if let guards become stable, which would allow us to do the best of both worlds!

@rnijveld rnijveld closed this May 8, 2023
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.

2 participants