-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Update encoding for read/subscribe/write request #11856
Update encoding for read/subscribe/write request #11856
Conversation
PR #11856: Size comparison from 09f9837 to 41c63a2 Increases above 0.2%:
Increases (21 builds for efr32, esp32, linux, mbed, nrfconnect, p6)
Decreases (31 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
Full report (38 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
|
What are the actual changes being made here? And is there a reason this is one big mega-PR instead of separate PRs for read, subscribe, write? Please keep in mind that review time goes about as O(N^2) of PR size unless the PR is just a search-and-replace... |
Agree, read and subscribe have several similar encoding stuff, so i made the change in the same PR... |
@yunhanw-google - merge conflicts |
PR #11856: Size comparison from e568d70 to ee6bf0f Increases above 0.2%:
Increases (18 builds for efr32, k32w, linux, p6, qpg)
Decreases (19 builds for efr32, k32w, linux, p6, qpg, telink)
Full report (21 builds for efr32, k32w, linux, p6, qpg, telink)
|
ee6bf0f
to
2e370f1
Compare
PR #11856: Size comparison from e568d70 to 2e370f1 Increases above 0.2%:
Increases (26 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg)
Decreases (31 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
Full report (38 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
|
I should note that this did not answer the question I actually asked.... |
Problem
update encoding for IM read/subscribe/write request and corresponding client/server
Change overview
See above
Testing
Update the existing unit tests to cover changes