Convert Inet::InterfaceId to a class #10979
Merged
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
#### Problem The current `#if`+`typedef` requires the actual definition to be visible at every appearance, rather than merely an opaque forward class declaration, which is an obstace to project-chip#7715 _Virtualize System and Inet interfaces_. #### Change overview Convert `InterfaceId` to a class. Some free functions become class methods, and `INET_NULL_INTERFACEID` is replaced by a default-constructed `InterfaceId`. #### Testing CI; no changes to functionality intended.
|
PR #10979: Size comparison from 596d838 to a9b96b8 Increases above 0.25%:
Increases (32 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
Decreases (2 builds for nrfconnect)
Full report (36 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
|
|
Size increase report for "gn_qpg-example-build" from 596d838
Full report output |
andy31415
approved these changes
Oct 26, 2021
kpschoedel
added a commit
to kpschoedel/connectedhomeip
that referenced
this pull request
Oct 26, 2021
#### Problem It's hard to pick out actual size changes, beyond those called out in the large-increase tables. Report information can be distributed across multiple collapsed tables, since the generator often runs before all builds are complete. #### Change overview Read back any existing comment and merge in new reports. Comments now contain: 1. An open table highlighting large increases, if any. 2. A collapsed table of all increases, if any. 3. A collapsed table of all decreases, if any. 4. A collapsed table of all data. #### Testing Tested with manual runs: project-chip#10979 (comment)
pullapprove
bot
requested review from
anush-apple,
balducci-apple,
bzbarsky-apple,
carol-apple,
chrisdecenzo,
chulspro,
Damian-Nordic,
franck-apple,
hawk248,
jelderton,
jepenven-silabs,
jmartinez-silabs,
LuDuda,
msandstedt,
pan-apple,
sagar-apple,
saurabhst,
selissia and
tecimovic
pullapprove
bot
requested review from
vivien-apple,
wbschiller and
woody-apple
woody-apple
pushed a commit
that referenced
this pull request
Oct 26, 2021
#### Problem It's hard to pick out actual size changes, beyond those called out in the large-increase tables. Report information can be distributed across multiple collapsed tables, since the generator often runs before all builds are complete. #### Change overview Read back any existing comment and merge in new reports. Comments now contain: 1. An open table highlighting large increases, if any. 2. A collapsed table of all increases, if any. 3. A collapsed table of all decreases, if any. 4. A collapsed table of all data. #### Testing Tested with manual runs: #10979 (comment)
|
Size increase report for "nrfconnect-example-build" from 596d838
Full report output |
|
Size increase report for "esp32-example-build" from 596d838
Full report output |
|
PR #10979: Size comparison from 596d838 to 2a45313 36 builds (for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
|
|
PR #10979: Size comparison from 596d838 to dc615cd Increases (32 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
Decreases (5 builds for linux, nrfconnect)
Full report (36 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
|
woody-apple
approved these changes
Oct 27, 2021
|
fast track: refactor without functionality change. A large amount of changes were within unit tests and CI test coverage should be quite extensive for this. |
andy31415
added a commit
to andy31415/connectedhomeip
that referenced
this pull request
Oct 27, 2021
andy31415
added a commit
that referenced
this pull request
Oct 27, 2021
JasonLiuZhuoCheng
pushed a commit
to JasonLiuZhuoCheng/connectedhomeip
that referenced
this pull request
Oct 28, 2021
#### Problem It's hard to pick out actual size changes, beyond those called out in the large-increase tables. Report information can be distributed across multiple collapsed tables, since the generator often runs before all builds are complete. #### Change overview Read back any existing comment and merge in new reports. Comments now contain: 1. An open table highlighting large increases, if any. 2. A collapsed table of all increases, if any. 3. A collapsed table of all decreases, if any. 4. A collapsed table of all data. #### Testing Tested with manual runs: project-chip#10979 (comment)
JasonLiuZhuoCheng
pushed a commit
to JasonLiuZhuoCheng/connectedhomeip
that referenced
this pull request
Oct 28, 2021
* Convert Inet::InterfaceId to a class #### Problem The current `#if`+`typedef` requires the actual definition to be visible at every appearance, rather than merely an opaque forward class declaration, which is an obstace to project-chip#7715 _Virtualize System and Inet interfaces_. #### Change overview Convert `InterfaceId` to a class. Some free functions become class methods, and `INET_NULL_INTERFACEID` is replaced by a default-constructed `InterfaceId`. #### Testing CI; no changes to functionality intended. * add InterfaceId::Null() * replace strcpy()
JasonLiuZhuoCheng
pushed a commit
to JasonLiuZhuoCheng/connectedhomeip
that referenced
this pull request
Oct 28, 2021
carol-apple
pushed a commit
to carol-apple/connectedhomeip
that referenced
this pull request
Oct 28, 2021
* Convert Inet::InterfaceId to a class #### Problem The current `#if`+`typedef` requires the actual definition to be visible at every appearance, rather than merely an opaque forward class declaration, which is an obstace to project-chip#7715 _Virtualize System and Inet interfaces_. #### Change overview Convert `InterfaceId` to a class. Some free functions become class methods, and `INET_NULL_INTERFACEID` is replaced by a default-constructed `InterfaceId`. #### Testing CI; no changes to functionality intended. * add InterfaceId::Null() * replace strcpy()
carol-apple
pushed a commit
to carol-apple/connectedhomeip
that referenced
this pull request
Oct 28, 2021
PSONALl
pushed a commit
to PSONALl/connectedhomeip
that referenced
this pull request
Dec 3, 2021
* Convert Inet::InterfaceId to a class #### Problem The current `#if`+`typedef` requires the actual definition to be visible at every appearance, rather than merely an opaque forward class declaration, which is an obstace to project-chip#7715 _Virtualize System and Inet interfaces_. #### Change overview Convert `InterfaceId` to a class. Some free functions become class methods, and `INET_NULL_INTERFACEID` is replaced by a default-constructed `InterfaceId`. #### Testing CI; no changes to functionality intended. * add InterfaceId::Null() * replace strcpy()
PSONALl
pushed a commit
to PSONALl/connectedhomeip
that referenced
this pull request
Dec 3, 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.
Problem
The current
#if+typedefrequires the actual definition to be visibleat every appearance, rather than merely an opaque forward class declaration,
which is an obstace to #7715 Virtualize System and Inet interfaces.
Change overview
Convert
InterfaceIdto a class. Some free functions become class methods,and
INET_NULL_INTERFACEIDis replaced by a default-constructedInterfaceId.Testing
CI; no changes to functionality intended.