Skip to content
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

Various: Make error messages more idiomatic #74

Merged
merged 1 commit into from
May 21, 2021
Merged

Various: Make error messages more idiomatic #74

merged 1 commit into from
May 21, 2021

Conversation

vilgotf
Copy link
Contributor

@vilgotf vilgotf commented May 20, 2021

I recently read the std::error::Error trait documentation and found that messages should (in general) be lowercase. After more searching I also found this which talks about how std::error::Error::source should behave (to produce nice error chains).

This pull requests implements these lessons for most error types (I didn't want to touch some of songbird::error::ConnectionError's errors (like songbird::ws::Error) due to their complexity).

Tested with cargo make ready

@vilgotf
Copy link
Contributor Author

vilgotf commented May 20, 2021

tracing_subscriber dosn't support multiple Error::source() OOTB tokio-rs/tracing#1347

@FelixMcFelix FelixMcFelix added the enhancement New feature or request label May 20, 2021
Copy link
Member

@FelixMcFelix FelixMcFelix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, these definitely agree with style/usability guidelines more than before.

@FelixMcFelix FelixMcFelix merged commit 55e59c2 into serenity-rs:next May 21, 2021
FelixMcFelix pushed a commit to FelixMcFelix/songbird that referenced this pull request May 21, 2021
@vilgotf vilgotf deleted the err branch June 6, 2021 19:53
FelixMcFelix pushed a commit to FelixMcFelix/songbird that referenced this pull request Jun 14, 2021
FelixMcFelix pushed a commit to FelixMcFelix/songbird that referenced this pull request Jul 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants