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

feat: Switch &str params #67

Open
HarryET opened this issue Dec 22, 2022 · 4 comments
Open

feat: Switch &str params #67

HarryET opened this issue Dec 22, 2022 · 4 comments

Comments

@HarryET
Copy link
Contributor

HarryET commented Dec 22, 2022

Stop using &str with lifetime for the params and instead use impl Into<String>

@MohammadAbuzar945
Copy link

Using impl Into<String> allows you to accept any type that can be converted into a String. This can be more flexible than using &str with a lifetime, because it allows the caller to pass in a variety of types, including owned Strings, &String, &str, and even types like String, Vec, and Box that implement the Into trait.

@HarryET
Copy link
Contributor Author

HarryET commented Jan 4, 2023

Yes, that is why we have this issue open to make that change, is it something you want to do @MohammadAbuzar945?

@rex-remind101
Copy link

@HarryET This would imply removing the borrow parameter 'a and moving to using String in Payload correct? https://github.com/WalletConnect/a2/blob/0b29470b6d559a9b3ae8d7136151b0a8e32f60b3/src/request/payload.rs#L10

@HarryET
Copy link
Contributor Author

HarryET commented Jul 1, 2023

@HarryET This would imply removing the borrow parameter 'a and moving to using String in Payload correct?

https://github.com/WalletConnect/a2/blob/0b29470b6d559a9b3ae8d7136151b0a8e32f60b3/src/request/payload.rs#L10

Correct, it should use String internally and all params should be S: Into<String> as a generic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants