Stefan Tauner has posted comments on this change. ( https://review.coreboot.org/21828 )
Change subject: fixup! Convert flashrom to git
......................................................................
Patch Set 1:
> Is there any automatic tool that checks for compliance? Or did
> anybody
> really check every line? How is adding this comment a fixup?
>
> I agree to the full-stop, btw. Line could be broken too, though.
>
> Is there any automatic tool that checks for compliance? Or did
> anybody
> really check every line? How is adding this comment a fixup?
>
> I agree to the full-stop, btw. Line could be broken too, though.
This aspect was brought up during the initial review and so for me it naturally belongs to that change. I think we need to write down what fixup patches in the staging branch are supposed to be at some time... :)
Independent from the general issue with the staging process I really want to get the git commit "complete" so we have one definitive starting point for all commits to come. Unfortunately this was not possible with staging itself but it should definitely be for any stable release branch (I do intend to have a 0.9.9.1 with the most important fixes to ease getting them in LTS distributions). Hence I want to have such rather benign things in it if possible.
The comment in question documents intent to make this clear for reviewers as contributors (while the sh shebang should be a clear hint I think the comment is still warranted due to the local restriction and its outspokenness). There was no strict review for this I guess but a) I always intended it to be that way (but of course there might be bugs... e.g., I wasn't aware local is not POSIX before you brought it up), b) you had a look for that obviously in the first review.
--
To view, visit https://review.coreboot.org/21828
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: staging
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia60186f783067ba084439a8ef701dc8f4c0072f0
Gerrit-Change-Number: 21828
Gerrit-PatchSet: 1
Gerrit-Owner: Stefan Tauner <stefan.tauner(a)gmx.at>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Mon, 02 Oct 2017 12:36:21 +0000
Gerrit-HasComments: No
Stefan Tauner has posted comments on this change. ( https://review.coreboot.org/21829 )
Change subject: fixup! Convert flashrom to git
......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/21829/1/util/getrevision.sh
File util/getrevision.sh:
https://review.coreboot.org/#/c/21829/1/util/getrevision.sh@a162
PS1, Line 162:
> Why drop that information? I can't see any change in behaviour?
That's a mistake, thanks. In my patches the line was even longer due to the elaborate version string... I did not read carefully and apparently removed too much here.
--
To view, visit https://review.coreboot.org/21829
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: staging
Gerrit-MessageType: comment
Gerrit-Change-Id: I6689ac24077b3981b471ed69de7cc3ef79d435b1
Gerrit-Change-Number: 21829
Gerrit-PatchSet: 1
Gerrit-Owner: Stefan Tauner <stefan.tauner(a)gmx.at>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Mon, 02 Oct 2017 12:26:21 +0000
Gerrit-HasComments: Yes
Nico Huber has posted comments on this change. ( https://review.coreboot.org/21829 )
Change subject: fixup! Convert flashrom to git
......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/21829/1/util/getrevision.sh
File util/getrevision.sh:
https://review.coreboot.org/#/c/21829/1/util/getrevision.sh@a162
PS1, Line 162:
Why drop that information? I can't see any change in behaviour?
(line should be broken to fit 80x25 terminal ofc, in case)
--
To view, visit https://review.coreboot.org/21829
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: staging
Gerrit-MessageType: comment
Gerrit-Change-Id: I6689ac24077b3981b471ed69de7cc3ef79d435b1
Gerrit-Change-Number: 21829
Gerrit-PatchSet: 1
Gerrit-Owner: Stefan Tauner <stefan.tauner(a)gmx.at>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Mon, 02 Oct 2017 12:20:37 +0000
Gerrit-HasComments: Yes
Nico Huber has posted comments on this change. ( https://review.coreboot.org/21828 )
Change subject: fixup! Convert flashrom to git
......................................................................
Patch Set 1:
Is there any automatic tool that checks for compliance? Or did anybody
really check every line? How is adding this comment a fixup?
I agree to the full-stop, btw. Line could be broken too, though.
--
To view, visit https://review.coreboot.org/21828
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: staging
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia60186f783067ba084439a8ef701dc8f4c0072f0
Gerrit-Change-Number: 21828
Gerrit-PatchSet: 1
Gerrit-Owner: Stefan Tauner <stefan.tauner(a)gmx.at>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Mon, 02 Oct 2017 11:57:53 +0000
Gerrit-HasComments: No
Nico Huber has posted comments on this change. ( https://review.coreboot.org/21827 )
Change subject: fixup! Convert flashrom to git
......................................................................
Patch Set 1:
(1 comment)
Don't see that the hidden behavioural change deserves a fixup. Let
me know if you have strong feelings about that.
https://review.coreboot.org/#/c/21827/1/Makefile
File Makefile:
https://review.coreboot.org/#/c/21827/1/Makefile@554
PS1, Line 554: $(shell ./util/getrevision.sh -c 2>/dev/null && ./util/git-hooks/install.sh)
The only behavioural change I can spot is that this is now also
done when both Makefile.VCS and git are available.
--
To view, visit https://review.coreboot.org/21827
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: staging
Gerrit-MessageType: comment
Gerrit-Change-Id: Iedc9df4c033a70447b8b1b65c83764c769b02c3f
Gerrit-Change-Number: 21827
Gerrit-PatchSet: 1
Gerrit-Owner: Stefan Tauner <stefan.tauner(a)gmx.at>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Mon, 02 Oct 2017 11:45:44 +0000
Gerrit-HasComments: Yes
Nico Huber has posted comments on this change. ( https://review.coreboot.org/21826 )
Change subject: fixup! Convert flashrom to git
......................................................................
Patch Set 1: Code-Review-2
(1 comment)
Not a fixup, pure cosmetic change. And I'm not convinced by the name
change. Makefile.VCS? Not only that it looks weird, I'd also have no
idea what to expect in there. It does neither provide a VCS nor is it
used when a VCS is available.
https://review.coreboot.org/#/c/21826/1//COMMIT_MSG
Commit Message:
https://review.coreboot.org/#/c/21826/1//COMMIT_MSG@11
PS1, Line 11: makefile change date but even this later data is not a "version".
It's more than a version, I have to admit that. But it's
not a VCS either.
--
To view, visit https://review.coreboot.org/21826
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: staging
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie57b20dc014ba44ded5783bdb432eb7d0e0e28ad
Gerrit-Change-Number: 21826
Gerrit-PatchSet: 1
Gerrit-Owner: Stefan Tauner <stefan.tauner(a)gmx.at>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Mon, 02 Oct 2017 11:39:26 +0000
Gerrit-HasComments: Yes
Nico Huber has posted comments on this change. ( https://review.coreboot.org/21825 )
Change subject: fixup! Convert flashrom to git
......................................................................
Patch Set 1:
I have a strong preference, indeed. But I can't force anybody as there
is no written rule. My general impression is: When somebody makes the
visible width (end minus indentation) of a text block wider than 80
chars, he doesn't care if somebody will read it. If he makes it wider
than 100 chars, he doesn't want it to be read.
The typesetting rule is to break around 60~70 chars (going back to
typesetting research done some centuries ago, iirc).
--
To view, visit https://review.coreboot.org/21825
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: staging
Gerrit-MessageType: comment
Gerrit-Change-Id: Iff4021be49a9fab208b619c555b9f9e81f671ab8
Gerrit-Change-Number: 21825
Gerrit-PatchSet: 1
Gerrit-Owner: Stefan Tauner <stefan.tauner(a)gmx.at>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Mon, 02 Oct 2017 11:31:26 +0000
Gerrit-HasComments: No
Stefan Tauner has posted comments on this change. ( https://review.coreboot.org/21838 )
Change subject: Fix standalone ich_descriptor_tool compilation with MinGW and DJGPP
......................................................................
Patch Set 1:
NB: this fixes compilation with MinGW that's broken since fa62294 (ich_descriptors: Update for Intel Skylake)
--
To view, visit https://review.coreboot.org/21838
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: staging
Gerrit-MessageType: comment
Gerrit-Change-Id: Id146f5ba06a0e510397c6f32a2bd7c819a405a25
Gerrit-Change-Number: 21838
Gerrit-PatchSet: 1
Gerrit-Owner: Stefan Tauner <stefan.tauner(a)gmx.at>
Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Mon, 02 Oct 2017 02:55:14 +0000
Gerrit-HasComments: No