-
Notifications
You must be signed in to change notification settings - Fork 640
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
Scannerctl structopt derive and code cleanup #1830
base: main
Are you sure you want to change the base?
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice. OpenSSF Scorecard
Scanned Files
|
1953cee
to
baa84fe
Compare
baa84fe
to
90a1093
Compare
🔍 Vulnerabilities of
|
digest | sha256:517d9d79825b0657b361ef83114317c00b1bfd700c750c145119fce6b41c8b27 |
vulnerabilities | |
size | 144 MB |
packages | 261 |
📦 Base Image debian:stable-20250113-slim
also known as |
|
digest | sha256:9dfddad9f09eadd2541a567e0865bd223387cf490b1c8d9d1f08d3b413766841 |
vulnerabilities |
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
Description
Description
Description
Description
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
Description
Description
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
Description
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
Description
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
Description
Description
Description | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
|
The initial idea of the errors within scannerctl was to have proper output for the user. For an example: Since we usually have all the information required in the local context I think panic could be a solution, however I also think panics are hard to verify while Result<(), Box> could be verified very easily. |
I'm not sure I follow that logic. All of the information required to generate that kind of message (with the filename possibly being an exception) should be available at wherever the As a result, we end up paying quite a large cost in terms of additional boilerplate, only to then give up all of the flexibility that this gives us and to basically unwrap our error type at the last possible point in the program. This also means that tracking down an individual unhelpful error message (e.g. the classic "No such file or directory") becomes especially cumbersome, since one has to do it at runtime, going down the call chain function by function. In an approach where we panic directly (or much closer to the actual error message), the stacktrace would give us a good pointer as to where the error came from. |
Use the structopt derive variant of clap instead of the arg builder for scannerctl.
As a result, delete lots of code. Since that was one of the fears: Compile times have basically not changed for me.
Also, I began cleaning up the all-over-the-place error handling in scannerctl, by making the
filename
inCliError
non mandatory, since very few of the errors actually use it and of those that do, many use it for something entirely different. There is still a lot left to be desired for the error handling here. In principle, I don't really see a need to do "proper" error handling in scannerctl anyways, since we build up some complicated error structure only to unwrap it at the end anways. My preferred method would be either panicking directly wherever the errors occur or constructing some kind of local error enum for each of the subcommands which just represents the various error messages and then unwrapping that.