Attention is currently required from: Karthik Ramasubramanian, Subrata Banik.
Jérémy Compostella has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/86225?usp=email )
Change subject: vc/google/chromeos: Add low battery indicator screen ......................................................................
Patch Set 2:
(1 comment)
File src/vendorcode/google/chromeos/Makefile.mk:
https://review.coreboot.org/c/coreboot/+/86225/comment/faf80f63_dd0cd4c9?usp... : PS1, Line 43: cbfs-files-$(CONFIG_CHROMEOS_LOW_BATTERY_INDICATOR_SCREEN) += low_battery_logo.bmp
This is the second time this code is duplicated. I would suggest to introduce a function. […]
In the provided code snippet, a repetitive pattern can be observed across three blocks of code, with only minor differences between them.
```makefile cbfs-files-$(CONFIG_CHROMEOS_FW_SPLASH_SCREEN) += cb_logo.bmp cb_logo.bmp-file := $(call strip_quotes,$(CONFIG_CHROMEOS_LOGO_PATH)) cb_logo.bmp-type := raw cb_logo.bmp-compression := $(BMP_LOGO_COMPRESS_FLAG)
cbfs-files-$(CONFIG_CHROMEOS_FW_SPLASH_SCREEN) += cb_plus_logo.bmp cb_plus_logo.bmp-file := $(call strip_quotes,$(CONFIG_CHROMEBOOK_PLUS_LOGO_PATH)) cb_plus_logo.bmp-type := raw cb_plus_logo.bmp-compression := $(BMP_LOGO_COMPRESS_FLAG)
cbfs-files-$(CONFIG_CHROMEOS_LOW_BATTERY_INDICATOR_SCREEN) += low_battery_logo.bmp low_battery_logo.bmp-file := $(call strip_quotes,$(CONFIG_CHROMEOS_LOW_BATTERY_INDICATOR_LOGO_PATH)) low_battery_logo.bmp-type := raw low_battery_logo.bmp-compression := $(BMP_LOGO_COMPRESS_FLAG) ```
To optimize and simplify the code, I recommend defining a macro function (untested) that encapsulates the common elements of these blocks. This macro can then be used to replace the repetitive code, enhancing maintainability and readability.
```makefile define define_logo_target cbfs-files-$(1) += $(2) $(2)-file := $$(call strip_quotes,$$($(3))) $(2)-type := raw $(2)-compression := $$(BMP_LOGO_COMPRESS_FLAG) endef ```