Attention is currently required from: Simon Buhrow, Thomas Heijligen, Aarya.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67715 )
Change subject: spi25.c: Move spi_get_opcode_from_erasefn() to spi.c
......................................................................
Patch Set 7: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/67715
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Id654e998d0af2d3f5845336bb98b38d724519038
Gerrit-Change-Number: 67715
Gerrit-PatchSet: 7
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Simon Buhrow
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Attention: Simon Buhrow
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Comment-Date: Sat, 10 Dec 2022 12:28:06 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Thomas Heijligen, Anastasia Klimchuk.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70176 )
Change subject: README: Add information about meson and link build instructions
......................................................................
Patch Set 2: Code-Review+2
(3 comments)
File README:
https://review.coreboot.org/c/flashrom/+/70176/comment/9150ddb1_8bd2b8f5
PS1, Line 49: Flashrom
> It is not painful to start with lowercase, so I did that :) […]
Sounds good, thank you very much!
https://review.coreboot.org/c/flashrom/+/70176/comment/c9d02348_baa27e20
PS1, Line 51: although not exactly
: all of them. Full meson support is on the roadmap in the nearest future.
> Yes, your suggestion makes sense, I added one more sentence/paragraph to guide the readers that make […]
Splendid! Even better than what we were thinking of!
https://review.coreboot.org/c/flashrom/+/70176/comment/d295f294_34b551f1
PS1, Line 54: /Documentation/building.md
> You will be delighted to know that on the last meeting of "One Build System Working Group" we decide […]
Delighted indeed! We're looking forward to this brainstorming session! :D
We agree with having this patch to unblock RC1. Finally, we're doing this!
--
To view, visit https://review.coreboot.org/c/flashrom/+/70176
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I2a27d8f2ba42e18be2485ae95bec1b4c874bb4f7
Gerrit-Change-Number: 70176
Gerrit-PatchSet: 2
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Sat, 10 Dec 2022 10:59:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Thomas Heijligen, Evan Benn.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69268 )
Change subject: tests: Add llvm-cov option and run target for code coverage
......................................................................
Patch Set 18: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/69268
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Id6c73bff46e7b88d425956a80def97082b201f56
Gerrit-Change-Number: 69268
Gerrit-PatchSet: 18
Gerrit-Owner: Evan Benn <evanbenn(a)google.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Evan Benn <evanbenn(a)google.com>
Gerrit-Comment-Date: Sat, 10 Dec 2022 08:50:19 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70006 )
Change subject: layout: Check return values for strdup in register_include_arg
......................................................................
Patch Set 8:
(1 comment)
File layout.c:
https://review.coreboot.org/c/flashrom/+/70006/comment/395697d2_d9cad3d7
PS8, Line 140: msg_gerr("Out of memory");
> I was advised by my reviewers to adjust new code to CB:69472 and so I opened that patch and looked i […]
CB:70552
--
To view, visit https://review.coreboot.org/c/flashrom/+/70006
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I6c22196be6847a8c9704f1de936604a51b4b8a28
Gerrit-Change-Number: 70006
Gerrit-PatchSet: 8
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Sat, 10 Dec 2022 08:44:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Thomas Heijligen, Angel Pons.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70176 )
Change subject: README: Add information about meson and link build instructions
......................................................................
Patch Set 2:
(3 comments)
File README:
https://review.coreboot.org/c/flashrom/+/70176/comment/868047e1_4966e9b9
PS1, Line 49: Flashrom
> nit: `flashrom` should not capitalised. […]
It is not painful to start with lowercase, so I did that :)
About rephrasing: I think the target audience would prefer direct sentences (I definitely do prefer direct). So leaving as is, just first letter lowercase.
https://review.coreboot.org/c/flashrom/+/70176/comment/dbc246e5_1f15bcaa
PS1, Line 51: although not exactly
: all of them. Full meson support is on the roadmap in the nearest future.
> Hmmm, if meson support isn't full-featured yet, should we explicitly suggest using make whenever pos […]
Yes, your suggestion makes sense, I added one more sentence/paragraph to guide the readers that make is still default.
There is a category of developers/users who are building with meson already (for years) and those who know what they are doing will just continue.
However, for the rest of people, I added clear guideline to use make for now, so that people are not confused.
https://review.coreboot.org/c/flashrom/+/70176/comment/6b399034_1b3fc64a
PS1, Line 54: /Documentation/building.md
> Should we convert the README into a README. […]
You will be delighted to know that on the last meeting of "One Build System Working Group" we decided we need to upgrade README to be fully relevant, up-to-date, readable, and crystal clear to the readers! :)
This of course requires a real brainstorming session, open to everyone who is interested, where we can brainstorm the better README together. I am looking forward to it.
Meanwhile, this patch is meant to unblock RC1.
--
To view, visit https://review.coreboot.org/c/flashrom/+/70176
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I2a27d8f2ba42e18be2485ae95bec1b4c874bb4f7
Gerrit-Change-Number: 70176
Gerrit-PatchSet: 2
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Sat, 10 Dec 2022 08:31:32 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Thomas Heijligen, Anastasia Klimchuk.
Hello build bot (Jenkins), David Hendricks, Thomas Heijligen, Edward O'Callaghan, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/70176
to look at the new patch set (#2).
Change subject: README: Add information about meson and link build instructions
......................................................................
README: Add information about meson and link build instructions
The patch adds one paragraph of information about meson into the
README file. This meant to be the minimum required to unblock
release candidate. README file will have a more substantial
upgrade soon.
Ticket: https://ticket.coreboot.org/issues/354
Change-Id: I2a27d8f2ba42e18be2485ae95bec1b4c874bb4f7
Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org>
---
M README
1 file changed, 29 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/76/70176/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/70176
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I2a27d8f2ba42e18be2485ae95bec1b4c874bb4f7
Gerrit-Change-Number: 70176
Gerrit-PatchSet: 2
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: newpatchset
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70006 )
Change subject: layout: Check return values for strdup in register_include_arg
......................................................................
Patch Set 8:
(2 comments)
File layout.c:
https://review.coreboot.org/c/flashrom/+/70006/comment/ac64dc66_23fab64a
PS8, Line 140: msg_gerr("Out of memory");
> Missing \n
I was advised by my reviewers to adjust new code to CB:69472 and so I opened that patch and looked into layout.c https://review.coreboot.org/c/flashrom/+/69472/3/layout.c
and did the same for my code.
Now after your question, I looked at that patch again, it actually has two files: layout.c and flashrom.c. The former has no `\n`, the latter has `\n` :D
I will create another patch to make layout.c out of memory error messages consistent.
https://review.coreboot.org/c/flashrom/+/70006/comment/3de9dfb4_b668c31a
PS8, Line 146: msg_gerr("Out of memory");
> Missing \n
Same as above, I will close this comment and keep another thread.
--
To view, visit https://review.coreboot.org/c/flashrom/+/70006
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I6c22196be6847a8c9704f1de936604a51b4b8a28
Gerrit-Change-Number: 70006
Gerrit-PatchSet: 8
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Sat, 10 Dec 2022 07:39:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net>
Gerrit-MessageType: comment