Hello Edward O'Callaghan, David Hendricks,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/flashrom/+/36909
to review the following change.
Change subject: Revert "print.c: Dedup 'test_state_to_text()' logic" ......................................................................
Revert "print.c: Dedup 'test_state_to_text()' logic"
This reverts commit 61e16e549a52194ac80ef40504f2dc661d1ff99c.
Obviously throws alignment in the table off and changes output class from `general` to `programmer` for no visible reason.
Change-Id: I864044b9fac6af9cf6a89c053eccdcb36f17c7bd Signed-off-by: Nico Huber nico.h@gmx.de --- M print.c 1 file changed, 8 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/09/36909/1
diff --git a/print.c b/print.c index 596fc53..6a7ff5d 100644 --- a/print.c +++ b/print.c @@ -401,7 +401,14 @@ for (i = 0; i < maxboardlen - strlen(b->name); i++) msg_ginfo(" ");
- msg_pinfo(test_state_to_text(b->working)); + switch (b->working) { + case OK: msg_ginfo("OK "); break; + case NT: msg_ginfo("NT "); break; + case DEP: msg_ginfo("DEP "); break; + case NA: msg_ginfo("N/A "); break; + case BAD: + default: msg_ginfo("BAD "); break; + }
for (e = board_matches; e->vendor_name != NULL; e++) { if (strcmp(e->vendor_name, b->vendor)
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/36909 )
Change subject: Revert "print.c: Dedup 'test_state_to_text()' logic" ......................................................................
Patch Set 1: Code-Review+2
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/36909 )
Change subject: Revert "print.c: Dedup 'test_state_to_text()' logic" ......................................................................
Patch Set 1:
Ah err, was this a problem I didn't realise the formatting wanted to be like this here?
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/36909 )
Change subject: Revert "print.c: Dedup 'test_state_to_text()' logic" ......................................................................
Patch Set 1:
Ah err, was this a problem I didn't realise the formatting wanted to be like this here?
Not sure if I understand your question. Yes, the formatting wants to be like this.
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/36909 )
Change subject: Revert "print.c: Dedup 'test_state_to_text()' logic" ......................................................................
Patch Set 1: Code-Review+2
David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/36909 )
Change subject: Revert "print.c: Dedup 'test_state_to_text()' logic" ......................................................................
Patch Set 1: Code-Review+2
I feel like there ought to be a comment or something there indicating that the formatting of the table is very fragile.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/36909 )
Change subject: Revert "print.c: Dedup 'test_state_to_text()' logic" ......................................................................
Patch Set 1:
Patch Set 1: Code-Review+2
I feel like there ought to be a comment or something there indicating that the formatting of the table is very fragile.
I agree, and/or a forward fix that just concat's a \t on to resolve this issue?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/36909 )
Change subject: Revert "print.c: Dedup 'test_state_to_text()' logic" ......................................................................
Patch Set 1: Code-Review+2
Have some moar green, even if I despise vegetables.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/36909 )
Change subject: Revert "print.c: Dedup 'test_state_to_text()' logic" ......................................................................
Patch Set 1:
Would this be a suitable forward fix perhaps instead?
https://review.coreboot.org/c/flashrom/+/37372
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/36909 )
Change subject: Revert "print.c: Dedup 'test_state_to_text()' logic" ......................................................................
Patch Set 1:
Can we get this in someday?
Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/36909 )
Change subject: Revert "print.c: Dedup 'test_state_to_text()' logic" ......................................................................
Revert "print.c: Dedup 'test_state_to_text()' logic"
This reverts commit 61e16e549a52194ac80ef40504f2dc661d1ff99c.
Obviously throws alignment in the table off and changes output class from `general` to `programmer` for no visible reason.
Change-Id: I864044b9fac6af9cf6a89c053eccdcb36f17c7bd Signed-off-by: Nico Huber nico.h@gmx.de Reviewed-on: https://review.coreboot.org/c/flashrom/+/36909 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Edward O'Callaghan quasisec@chromium.org Reviewed-by: HAOUAS Elyes ehaouas@noos.fr Reviewed-by: David Hendricks david.hendricks@gmail.com Reviewed-by: Angel Pons th3fanbus@gmail.com --- M print.c 1 file changed, 8 insertions(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified David Hendricks: Looks good to me, approved HAOUAS Elyes: Looks good to me, approved Angel Pons: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved
diff --git a/print.c b/print.c index 596fc53..6a7ff5d 100644 --- a/print.c +++ b/print.c @@ -401,7 +401,14 @@ for (i = 0; i < maxboardlen - strlen(b->name); i++) msg_ginfo(" ");
- msg_pinfo(test_state_to_text(b->working)); + switch (b->working) { + case OK: msg_ginfo("OK "); break; + case NT: msg_ginfo("NT "); break; + case DEP: msg_ginfo("DEP "); break; + case NA: msg_ginfo("N/A "); break; + case BAD: + default: msg_ginfo("BAD "); break; + }
for (e = board_matches; e->vendor_name != NULL; e++) { if (strcmp(e->vendor_name, b->vendor)