-
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
Move the isArray logic into the decodable/encodable type helper. #10822
Merged
bzbarsky-apple
merged 2 commits into
project-chip:master
from
bzbarsky-apple:nicer-decodable-encodable-type
Oct 23, 2021
Merged
Move the isArray logic into the decodable/encodable type helper. #10822
bzbarsky-apple
merged 2 commits into
project-chip:master
from
bzbarsky-apple:nicer-decodable-encodable-type
Oct 23, 2021
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yunhanw-google
approved these changes
Oct 22, 2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
PR #10822: Size comparison from 0c16976 to 64d258b 8 builds (for k32w, p6, qpg, telink)
14 builds (for efr32, linux, mbed)
|
bzbarsky-apple
force-pushed
the
nicer-decodable-encodable-type
branch
from
October 22, 2021 03:01
64d258b
to
055f407
Compare
pullapprove
bot
requested review from
anush-apple,
balducci-apple,
carol-apple,
chrisdecenzo,
chulspro,
Damian-Nordic,
franck-apple,
hawk248,
jelderton,
jepenven-silabs,
jmartinez-silabs,
LuDuda,
msandstedt,
pan-apple,
sagar-apple,
saurabhst,
wbschiller and
woody-apple
October 22, 2021 03:02
PR #10822: Size comparison from 0c16976 to 055f407 8 builds (for k32w, p6, qpg, telink)
14 builds (for efr32, linux, mbed)
12 builds (for esp32, nrfconnect)
|
Vivien had some suggestions that I'll implement. |
bzbarsky-apple
force-pushed
the
nicer-decodable-encodable-type
branch
from
October 22, 2021 16:08
055f407
to
e3919ea
Compare
This way instead of ugly templating to wrap in List/DecodableList it just works automatically.
PR #10822: Size comparison from 33ca522 to e3919ea 22 builds (for efr32, k32w, linux, mbed, p6, qpg, telink)
|
bzbarsky-apple
force-pushed
the
nicer-decodable-encodable-type
branch
from
October 22, 2021 16:42
e3919ea
to
b62200d
Compare
don't convert a bunch of integer args to const refs in ways that the rest of the system does not expect.
bzbarsky-apple
force-pushed
the
nicer-decodable-encodable-type
branch
from
October 22, 2021 16:53
b62200d
to
73d395f
Compare
woody-apple
approved these changes
Oct 22, 2021
PR #10822: Size comparison from 79cc55c to 73d395f 35 builds (for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
|
jmartinez-silabs
approved these changes
Oct 23, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This way instead of ugly templating to wrap in List/DecodableList we
can just pass in
isList=someBoolean
and have it all work.Problem
We have repetitive template logic to deal with lists that is a bit ugly to write.
Change overview
Move this logic into JS, make the templates cleaner. Soon we can also do nullables and optionals this way as well, I hope.
Testing
No changes to generated code.