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.

Sign up for GitHub

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

Jump to bottom

Chef - Don't use metadata for hash generation in naming convention #25885

Merged
merged 1 commit into from
Mar 30, 2023
Merged

Chef - Don't use metadata for hash generation in naming convention #25885

merged 1 commit into from
Mar 30, 2023

Conversation

cpagravel
Copy link
Contributor

@cpagravel cpagravel commented Mar 29, 2023 •
edited
Loading

The current naming convention for Chef sample apps is to use a hash which is generated by digesting the metadata information of the sample app. The initial idea was that it would be advantageous to relate the meta info to the hash to help identify the app and associate it with the config. This turns out not to be useful if the config files undergo frequent changes due to tooling changes like zapp updates or name changes within the spec.

There was no value added by using the complex meta file digestion to name the sample apps. It would be better to remove it and encourage the naming convention to be applied only upon creation of new sample apps. I.e. the naming convention should not be applied to previously created apps so we can maintain consistency for CI/CD testing pipelines.

@github-actions
Copy link

PR #25885: Size comparison from 2aefaa6 to c4ac97b

Decreases (1 build for cc32xx)
platform target config section 2aefaa6 c4ac97b change % change
cc32xx lock CC3235SF_LAUNCHXL .debug_info 20310044 20310043 -1 -0.0
Full report (1 build for cc32xx)
platform target config section 2aefaa6 c4ac97b change % change
cc32xx lock CC3235SF_LAUNCHXL 0 0 0 0.0
(read only) 645769 645769 0 0.0
(read/write) 203848 203848 0 0.0
.ARM.attributes 44 44 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 197248 197248 0 0.0
.comment 194 194 0 0.0
.data 1480 1480 0 0.0
.debug_abbrev 933102 933102 0 0.0
.debug_aranges 87704 87704 0 0.0
.debug_frame 301596 301596 0 0.0
.debug_info 20310044 20310043 -1 -0.0
.debug_line 2681024 2681024 0 0.0
.debug_loc 2827360 2827360 0 0.0
.debug_ranges 286352 286352 0 0.0
.debug_str 3041144 3041144 0 0.0
.ramVecs 780 780 0 0.0
.resetVecs 64 64 0 0.0
.rodata 105953 105953 0 0.0
.shstrtab 232 232 0 0.0
.stab 204 204 0 0.0
.stabstr 441 441 0 0.0
.stack 2048 2048 0 0.0
.strtab 380543 380543 0 0.0
.symtab 257472 257472 0 0.0
.text 537696 537696 0 0.0

@cpagravel cpagravel force-pushed the master branch 2 times, most recently from 0547296 to 0f0e701 Compare March 29, 2023 21:07
@github-actions
Copy link

PR #25885: Size comparison from 2aefaa6 to 0f0e701

Decreases (1 build for cc32xx)
platform target config section 2aefaa6 0f0e701 change % change
cc32xx lock CC3235SF_LAUNCHXL .debug_info 20310044 20310043 -1 -0.0
Full report (1 build for cc32xx)
platform target config section 2aefaa6 0f0e701 change % change
cc32xx lock CC3235SF_LAUNCHXL 0 0 0 0.0
(read only) 645769 645769 0 0.0
(read/write) 203848 203848 0 0.0
.ARM.attributes 44 44 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 197248 197248 0 0.0
.comment 194 194 0 0.0
.data 1480 1480 0 0.0
.debug_abbrev 933102 933102 0 0.0
.debug_aranges 87704 87704 0 0.0
.debug_frame 301596 301596 0 0.0
.debug_info 20310044 20310043 -1 -0.0
.debug_line 2681024 2681024 0 0.0
.debug_loc 2827360 2827360 0 0.0
.debug_ranges 286352 286352 0 0.0
.debug_str 3041144 3041144 0 0.0
.ramVecs 780 780 0 0.0
.resetVecs 64 64 0 0.0
.rodata 105953 105953 0 0.0
.shstrtab 232 232 0 0.0
.stab 204 204 0 0.0
.stabstr 441 441 0 0.0
.stack 2048 2048 0 0.0
.strtab 380543 380543 0 0.0
.symtab 257472 257472 0 0.0
.text 537696 537696 0 0.0

@github-actions
Copy link

PR #25885: Size comparison from 2aefaa6 to 626dc32

Full report (1 build for cc32xx)
platform target config section 2aefaa6 626dc32 change % change
cc32xx lock CC3235SF_LAUNCHXL 0 0 0 0.0
(read only) 645769 645769 0 0.0
(read/write) 203848 203848 0 0.0
.ARM.attributes 44 44 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 197248 197248 0 0.0
.comment 194 194 0 0.0
.data 1480 1480 0 0.0
.debug_abbrev 933102 933102 0 0.0
.debug_aranges 87704 87704 0 0.0
.debug_frame 301596 301596 0 0.0
.debug_info 20310044 20310044 0 0.0
.debug_line 2681024 2681024 0 0.0
.debug_loc 2827360 2827360 0 0.0
.debug_ranges 286352 286352 0 0.0
.debug_str 3041144 3041144 0 0.0
.ramVecs 780 780 0 0.0
.resetVecs 64 64 0 0.0
.rodata 105953 105953 0 0.0
.shstrtab 232 232 0 0.0
.stab 204 204 0 0.0
.stabstr 441 441 0 0.0
.stack 2048 2048 0 0.0
.strtab 380543 380543 0 0.0
.symtab 257472 257472 0 0.0
.text 537696 537696 0 0.0

@cpagravel
Copy link
Contributor Author

@gautesl did you work on the Sphinx documentation in #24185 ? If so, would you know why the CI test here is failing?

@github-actions
Copy link

PR #25885: Size comparison from 2aefaa6 to 39ef94f

Increases (1 build for nrfconnect)
platform target config section 2aefaa6 39ef94f change % change
nrfconnect all-clusters-minimal-app nrf52840dk_nrf52840 text 774352 774356 4 0.0
Full report (7 builds for cc32xx, mbed, nrfconnect, qpg)
platform target config section 2aefaa6 39ef94f change % change
cc32xx lock CC3235SF_LAUNCHXL 0 0 0 0.0
(read only) 645769 645769 0 0.0
(read/write) 203848 203848 0 0.0
.ARM.attributes 44 44 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 197248 197248 0 0.0
.comment 194 194 0 0.0
.data 1480 1480 0 0.0
.debug_abbrev 933102 933102 0 0.0
.debug_aranges 87704 87704 0 0.0
.debug_frame 301596 301596 0 0.0
.debug_info 20310044 20310044 0 0.0
.debug_line 2681024 2681024 0 0.0
.debug_loc 2827360 2827360 0 0.0
.debug_ranges 286352 286352 0 0.0
.debug_str 3041144 3041144 0 0.0
.ramVecs 780 780 0 0.0
.resetVecs 64 64 0 0.0
.rodata 105953 105953 0 0.0
.shstrtab 232 232 0 0.0
.stab 204 204 0 0.0
.stabstr 441 441 0 0.0
.stack 2048 2048 0 0.0
.strtab 380543 380543 0 0.0
.symtab 257472 257472 0 0.0
.text 537696 537696 0 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2466536 2466536 0 0.0
.bss 215964 215964 0 0.0
.data 5880 5880 0 0.0
.text 1429180 1429180 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read only) 4 4 0 0.0
(read/write) 1174112 1174112 0 0.0
bss 155557 155557 0 0.0
rodata 132640 132640 0 0.0
text 805148 805148 0 0.0
all-clusters-minimal-app nrf52840dk_nrf52840 (read only) 4 4 0 0.0
(read/write) 1119168 1119168 0 0.0
bss 154713 154713 0 0.0
rodata 109456 109456 0 0.0
text 774352 774356 4 0.0
all-clusters-app nrf7002dk_nrf5340_cpuapp (read only) 4 4 0 0.0
(read/write) 1434132 1434132 0 0.0
bss 135297 135297 0 0.0
rodata 228692 228692 0 0.0
text 776068 776068 0 0.0
qpg lighting-app qpg6105+debug (read/write) 1152952 1152952 0 0.0
.bss 96036 96036 0 0.0
.data 852 852 0 0.0
.text 600048 600048 0 0.0
lock-app qpg6105+debug (read/write) 1121528 1121528 0 0.0
.bss 91172 91172 0 0.0
.data 856 856 0 0.0
.text 568628 568628 0 0.0

@gautesl
Copy link
Contributor

gautesl commented Mar 30, 2023

@gautesl did you work on the Sphinx documentation in #24185 ? If so, would you know why the CI test here is failing?

Hi, an update to the doc dependancy pydata-sphinx-theme broke the build. A fix for it is suggested here #25893, and further doc issues preventing a successful build are proposed here #25894

@andy31415 andy31415 merged commit 3a80f8a into project-chip:master Mar 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bzbarsky-apple bzbarsky-apple bzbarsky-apple approved these changes

@amitnj amitnj Awaiting requested review from amitnj

@arkq arkq Awaiting requested review from arkq

@chrisdecenzo chrisdecenzo Awaiting requested review from chrisdecenzo

@chshu chshu Awaiting requested review from chshu

@chulspro chulspro Awaiting requested review from chulspro

@cliffamzn cliffamzn Awaiting requested review from cliffamzn

@CodeChronos928 CodeChronos928 Awaiting requested review from CodeChronos928

@Damian-Nordic Damian-Nordic Awaiting requested review from Damian-Nordic

@dhrishi dhrishi Awaiting requested review from dhrishi

@electrocucaracha electrocucaracha Awaiting requested review from electrocucaracha

@emargolis emargolis Awaiting requested review from emargolis

@gjc13 gjc13 Awaiting requested review from gjc13

@harsha-rajendran harsha-rajendran Awaiting requested review from harsha-rajendran

@hawk248 hawk248 Awaiting requested review from hawk248

@jelderton jelderton Awaiting requested review from jelderton

@jepenven-silabs jepenven-silabs Awaiting requested review from jepenven-silabs

@jmartinez-silabs jmartinez-silabs Awaiting requested review from jmartinez-silabs

@jmeg-sfy jmeg-sfy Awaiting requested review from jmeg-sfy

@joonhaengHeo joonhaengHeo Awaiting requested review from joonhaengHeo

@kcoppock kcoppock Awaiting requested review from kcoppock

@kkasperczyk-no kkasperczyk-no Awaiting requested review from kkasperczyk-no

@lazarkov lazarkov Awaiting requested review from lazarkov

@lpbeliveau-silabs lpbeliveau-silabs Awaiting requested review from lpbeliveau-silabs

@LuDuda LuDuda Awaiting requested review from LuDuda

@mrjerryjohns mrjerryjohns Awaiting requested review from mrjerryjohns

@msandstedt msandstedt Awaiting requested review from msandstedt

@mspang mspang Awaiting requested review from mspang

@robszewczyk robszewczyk Awaiting requested review from robszewczyk

@saurabhst saurabhst Awaiting requested review from saurabhst

@selissia selissia Awaiting requested review from selissia

@sharadb-amazon sharadb-amazon Awaiting requested review from sharadb-amazon

@tcarmelveilleux tcarmelveilleux Awaiting requested review from tcarmelveilleux

@tecimovic tecimovic Awaiting requested review from tecimovic

@tehampson tehampson Awaiting requested review from tehampson

@turon turon Awaiting requested review from turon

@vijs vijs Awaiting requested review from vijs

@xylophone21 xylophone21 Awaiting requested review from xylophone21

@younghak-hwang younghak-hwang Awaiting requested review from younghak-hwang

@andy31415 andy31415 Awaiting requested review from andy31415

Assignees
No one assigned
Labels
examples review - approved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@cpagravel @gautesl @andy31415 @bzbarsky-apple