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

refactor: make ErrorObject::borrowed accept &str #1160

Merged
merged 3 commits into from
Aug 10, 2023

Conversation

aj3n
Copy link
Contributor

@aj3n aj3n commented Jul 28, 2023

If you pass DST reference type as message to ErrorObject::borrowed, you'll get error like this:

error[E0277]: the size for values of type `str` cannot be known at compilation time
  --> src/main.rs:2:50
   |
2  |     jsonrpsee::types::ErrorObjectOwned::borrowed(0, "ok", None);
   |     --------------------------------------------    ^^^^ doesn't have a size known at compile-time
   |     |
   |     required by a bound introduced by this call
   |
   = help: the trait `Sized` is not implemented for `str`

It could be more intuitive to accept such usage.

@aj3n aj3n requested a review from a team as a code owner July 28, 2023 09:00
types/src/error.rs Outdated Show resolved Hide resolved
@niklasad1
Copy link
Member

niklasad1 commented Aug 9, 2023

@aj3n are you working on making the change as suggested in the code review?

I guess you may need to change a bunch of test code but would be good to merge this.

types/src/error.rs Outdated Show resolved Hide resolved
@niklasad1 niklasad1 changed the title make ErrorObject::borrowed accept DST message type make ErrorObject::borrowed accept &str Aug 10, 2023
@niklasad1 niklasad1 changed the title make ErrorObject::borrowed accept &str refactor: make ErrorObject::borrowed accept &str Aug 10, 2023
types/src/error.rs Outdated Show resolved Hide resolved
@niklasad1 niklasad1 added the breaking change Breaking change in the public APIs label Aug 10, 2023
@niklasad1 niklasad1 merged commit 1045057 into paritytech:master Aug 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Breaking change in the public APIs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants