reintroduces thiserror + improve output_to_wav error #778
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Looks great. I'm in favor. |
== Thiserror This reverts PR #633 in which thiserror was removed. At the time there where only two Error types. I expected that number not to increase. Clearly I was wrong as we now we have 6. Therefore this re-introduces thiserror. Note the original argument against this thiserror by est31 in 2021: > I'm not in favour of adding thiserror, it pulls in a huge set of dependencies for very little improvement in comfort. Since then thiserror has become *the* error handling crate for use in crates (24580 published crates depend on thiserror). The crate most used for error handling in binaries (anyhow) even relies on thiserror. Thiserror is also increadibly stable. Therefore the chance is really quite slim a rodio user will not already have the right thiserror version somewhere in their dependency tree. Which means the compile time impact is neglible in most cases. == Output to wav Gets its own type, it was `Box<dyn Error>` which does not work for wrapping it in anything requiring `Box<dyn Error + Send + Sync + 'static>`. Unfortunaly making wav_output return a `Box<dyn Error + Send + Sync + 'static>` will break usage in functions returning `Box<dyn Error>` (yeah this suprised me too). == Errors Clone This also makes all our errors clonable by swapping Box with Arc and wrapping non clone errors in an Arc. == Errors trait check macro Finally I've added a macro (`assert_error_traits`) in `common` to verify an Error type is Send + Sync + Clone + Debug + Display + 'static.
|
rebased and merged :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Thiserror
This reverts PR #633 in which thiserror was removed. At the time there where only two Error types. I expected that number not to increase. Clearly I was wrong as we now we have 6. Therefore this re-introduces thiserror.
Note the original argument against this thiserror by est31 in 2021:
Since then thiserror has become the error handling crate for use in crates (24580 published crates depend on thiserror). The crate most used for error handling in binaries (anyhow) even relies on thiserror. Thiserror is also increadibly stable.
Therefore the chance is really quite slim a rodio user will not already have the right thiserror version somewhere in their dependency tree. Which means the compile time impact is neglible in most cases.
Output to wav
Gets its own type, it was
Box<dyn Error>which does not work for wrapping it in anything requiringBox<dyn Error + Send + Sync + 'static>. Unfortunaly making wav_output return aBox<dyn Error + Send + Sync + 'static>will break usage in functions returningBox<dyn Error>(yeah this suprised me too).Errors Clone
This also makes all our errors clonable by swapping Box with Arc and wrapping non clone errors in an Arc.
Errors trait check macro
Finally I've added a macro (
assert_error_traits) incommonto verify an Error type is Send + Sync + Clone + Debug + Display + 'static.