Attention is currently required from: Aarya, Edward O'Callaghan, Peter Marheine.
Anastasia Klimchuk has posted comments on this change by Aarya. ( https://review.coreboot.org/c/flashrom/+/83833?usp=email )
Change subject: programmer.h: Introduce new enum for is_laptop ......................................................................
Patch Set 1:
(4 comments)
Patchset:
PS1: Aarya, I am very happy to see you here again! :) I added few comments , but I will look once more later. Thank you for working on this!
Commit Message:
https://review.coreboot.org/c/flashrom/+/83833/comment/e953034d_8d775421?usp... : PS1, Line 11: Maybe you can add link to the ticket in the commit message https://ticket.coreboot.org/issues/416
File include/programmer.h:
https://review.coreboot.org/c/flashrom/+/83833/comment/e7c651f7_e655df51?usp... : PS1, Line 167: enum is_laptop { I think, it's better to merge two patches together. What do you think? This enum by itself doesn't do much, but the next patch fully implements its purpose. I would prefer that in the same patch.
https://review.coreboot.org/c/flashrom/+/83833/comment/1dabf5f5_bdfb745b?usp... : PS1, Line 167: enum is_laptop { : NO, : YES, : UNKNOWN, : }; My first thought was that UNKNOWN would be a more intuitive default value. But then, I understand that this order keep the same order as it was with numbers (0 default was equivalent to NO). Does anyone has thoughts on better default value, NO or UNKNOWN?