Replace show image crate #55
Conversation
Alex Recommends ReportAlex recommends the following language changes, but Alex is a regular expression based algorithm, so take them with a grain of salt. ✨ 🚀 ✨ Nothing to Report ✨ 🚀 ✨ |
| window_title: &str, | ||
| window_width: u32, | ||
| window_height: u32, | ||
| ) -> Result<WindowCanvas, String> { |
There was a problem hiding this comment.
for rust libraries it's generally not recommended to use strings for the error type. this makes it hard to do anything but log the error. I think other places in Ibeji are using Status (e.g. here) so ideally we should use that here too
There was a problem hiding this comment.
I used String to be consistent with the sdl2 crate's methods, which use String.
Status comes from the tonic crate and is currently only used in conjunction with gRPC-related calls.
There was a problem hiding this comment.
I used String to be consistent with the sdl2 crate's methods, which use String.
Converting errors can be done with map_err. It's also worth mentioning that the library owner would prefer to use a proper error type but hasn't made the change due to concerns with breaking changes: Rust-SDL2/rust-sdl2#1053
Status comes from the tonic crate and is currently only used in conjunction with gRPC-related calls.
Then Status wouldn't be the right thing to use, we should find something else. Consider defining your own error type similar to what Freyja does, or you could use anyhow (not recommended for libraries but easy to use and better than String) or thiserror, or even reuse std::io::Error
* Replace show-image crate * Replace show-image crate * Replace show-image crate * Replace show-image crate * Replace show-image crate * Replace show-image crate * Replace show-image crate * Replace show-image crate * Replace show-image crate * Replace show-image crate * Replace show-image crate * Replace show-image crate * Replace show-image crate * Replace show-image crate * Replace show-image crate * Replace show-image crate * Replace show-image crate * Replace show-image crate * Replace show-image crate * Replace show-image crate * Replace show-image crate * Replace show-image crate
The show-image crate used an OSS license that we did not want to use. This fix moved us over to the sdl2 create, which uses OSS license that we are happy to use.