Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Intel coreboot Reviewers, Jakub "Kuba" Czapiga, Jayvik Desai, Jérémy Compostella, Kapil Porwal, Nick Vaccaro, Subrata Banik, Tarun.
Julius Werner has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/86283?usp=email )
Change subject: lib: Refactor ux_locales_get_text API
......................................................................
Patch Set 8:
(1 comment)
File src/include/ux_locales.h:
https://review.coreboot.org/c/coreboot/+/86283/comment/3fdc6ae6_832f5ac5?usp... :
PS5, Line 23: const char *ux_locales_get_text(const char *name, const char *fallback_text);
Well, then change the test, of course. […]
I don't think we should keep tests for an API that we do not want to expose for anyone to call anymore. Also, removing the other function may make the implementation of the `by_id()` function simpler (because then it will just be integrated into the main function and not be a separate thing), and it will also mean we would probably just call it `ux_locales_get_text()`, without the `_by_id`, because then that's the only option.
In general, the development flow should always start with deciding how you want the code to work at the high level, then designing the APIs that connect the different pieces, then implementing each of those individual parts, and then writing/adjusting unit tests for them. You should never turn that process around and e.g. have your decision of how to design an API influenced by how existing unit tests are written. The unit tests are there to support the code, not the other way round.
--
To view, visit
https://review.coreboot.org/c/coreboot/+/86283?usp=email
To unsubscribe, or for help writing mail filters, visit
https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I4952802396265b9ee8d164d6e43a7f2b3599d6c0
Gerrit-Change-Number: 86283
Gerrit-PatchSet: 8
Gerrit-Owner: Subrata Banik
subratabanik@google.com
Gerrit-Reviewer: Dinesh Gehlot
digehlot@google.com
Gerrit-Reviewer: Eran Mitrani
mitrani@google.com
Gerrit-Reviewer: Intel coreboot Reviewers
intel_coreboot_reviewers@intel.com
Gerrit-Reviewer: Jakub "Kuba" Czapiga
czapiga@google.com
Gerrit-Reviewer: Jayvik Desai
jayvik@google.com
Gerrit-Reviewer: Julius Werner
jwerner@chromium.org
Gerrit-Reviewer: Kapil Porwal
kapilporwal@google.com
Gerrit-Reviewer: Nick Vaccaro
nvaccaro@chromium.org
Gerrit-Reviewer: Tarun
tstuli@gmail.com
Gerrit-Reviewer: build bot (Jenkins)
no-reply@coreboot.org
Gerrit-CC: Jérémy Compostella
jeremy.compostella@intel.com
Gerrit-Attention: Jayvik Desai
jayvik@google.com
Gerrit-Attention: Intel coreboot Reviewers
intel_coreboot_reviewers@intel.com
Gerrit-Attention: Eran Mitrani
mitrani@google.com
Gerrit-Attention: Subrata Banik
subratabanik@google.com
Gerrit-Attention: Jakub "Kuba" Czapiga
czapiga@google.com
Gerrit-Attention: Jérémy Compostella
jeremy.compostella@intel.com
Gerrit-Attention: Kapil Porwal
kapilporwal@google.com
Gerrit-Attention: Dinesh Gehlot
digehlot@google.com
Gerrit-Attention: Nick Vaccaro
nvaccaro@chromium.org
Gerrit-Attention: Tarun
tstuli@gmail.com
Gerrit-Comment-Date: Sat, 08 Feb 2025 02:31:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik
subratabanik@google.com
Comment-In-Reply-To: Julius Werner
jwerner@chromium.org