chore: break method API #49
Conversation
| @@ -101,6 +101,13 @@ pub async fn handler( | |||
| increment_counter!(state.metrics, get_queries); | |||
| increment_counter_with!(state.metrics, served_items, messages.len() as u64); | |||
|
|
|||
| // Prematurely make breaking change of removing method | |||
| // https://walletconnect.slack.com/archives/C04CKNV4GN8/p1688127376863359 | |||
There was a problem hiding this comment.
Nit: I wouldn't put slack links into code
There was a problem hiding this comment.
What is your thinking? I figured good as a reference/context
There was a problem hiding this comment.
Add it to the PR description then, not the code.
There was a problem hiding this comment.
Will remove the Slack link 👍
There was a problem hiding this comment.
Isn't that a bit too soon?
We really need to make sure the SDKs won't need it before removing it.
How do they reconstruct history if they don't know if a message was sent or received, is it from the payload?
Also, I agree with @Elyniss, please remove the Slack link.
| @@ -101,6 +101,13 @@ pub async fn handler( | |||
| increment_counter!(state.metrics, get_queries); | |||
| increment_counter_with!(state.metrics, served_items, messages.len() as u64); | |||
|
|
|||
| // Prematurely make breaking change of removing method | |||
| // https://walletconnect.slack.com/archives/C04CKNV4GN8/p1688127376863359 | |||
There was a problem hiding this comment.
Add it to the PR description then, not the code.
This was discussed here, which I know you bumped again. Agree we need to make sure.
Not too soon if we know they don't need it. Better to remove now to make sure they don't need it, then attempt to launch Webhooks 2.0 later and realize you need it and need to go back to design board. |
Let's keep it in |
Description
Prematurely break method API as it is not needed by any SDKs: https://walletconnect.slack.com/archives/C04CKNV4GN8/p1688127376863359
This is in preparation for webhooks 2.0 where
methodis no longer sent: https://walletconnect.slack.com/archives/C05ABTQSPFY/p1688072990077729Doing now in order to avoid surprises later. And we can easily roll this back with minimal damage if we realize we needed it.
Also updated
proc-macro2just like I did over here.Resolves #48
How Has This Been Tested?
Not tested
Due Diligence