David Hendricks has posted comments on this change. ( https://review.coreboot.org/19206 )
Change subject: Convert flashrom to git ......................................................................
Patch Set 6:
(9 comments)
https://review.coreboot.org/#/c/19206/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/19206/6//COMMIT_MSG@13 PS6, Line 13: - Remove djgpp-dos target (it is not different to other x-compilations).
I was just wondering, why I didn't nag earlier, you can keep
Moved the change to another patch: https://review.coreboot.org/20293
https://review.coreboot.org/#/c/19206/6//COMMIT_MSG@15 PS6, Line 15: anything else from the code dump when we export the respective variable.
Nit, line is too long.
Done
https://review.coreboot.org/#/c/19206/6//COMMIT_MSG@26 PS6, Line 26: f5dd7ce1
This sha will probably get lost. Linking to the mailing list
It's actually the hash from the stable branch: https://review.coreboot.org/cgit/flashrom.git/commit/?id=f5dd7ce11b65ffd6ead...
I guess the mailing list is the definitive source for this kind of stuff though, so I'll link to https://mail.coreboot.org/pipermail/flashrom/2016-November/014877.html instead.
For newer patches we can use flashrom-gerrit (https://mail.coreboot.org/pipermail/flashrom-gerrit/) to cite the source of such patches.
https://review.coreboot.org/#/c/19206/6//COMMIT_MSG@28 PS6, Line 28: pushing to the review server.
Maybe also mention, that you split out the complex version
Done
https://review.coreboot.org/#/c/19206/6/util/getrevision.sh File util/getrevision.sh:
https://review.coreboot.org/#/c/19206/6/util/getrevision.sh@1 PS6, Line 1: #!/bin/sh We should probably either make this #!/bin/bash or remove local variables. That would be for another patch, though.
https://review.coreboot.org/#/c/19206/6/util/getrevision.sh@59 PS6, Line 59: and git-svn branches/tags removed the bit about git-svn
https://review.coreboot.org/#/c/19206/6/util/getrevision.sh@140 PS6, Line 140: local svn_base=$(git log --grep=git-svn-id -1 --format='%h') : if [ "$svn_base" != "" ] ; then : local diff_to_svn=$(git rev-list --count ${svn_base}..${r}) : if [ "$diff_to_svn" -gt 0 ] ; then : r="$r-$diff_to_svn" : fi : fi
Looks like we don't have git-svn-id any more. A simple `git describe
Done. I also took the liberty of reducing the indent level of this function by exiting earlier if the file isn't tracked.
https://review.coreboot.org/#/c/19206/6/util/getrevision.sh@168 PS6, Line 168: sed 's/.*@/r/;s/[[:blank:]].*//')
Same here, maybe just drop the --upstream command?
SGTM. It is a bit of a UI change, but AFAICT --upstream just returns "unknown" now so it's useless and isn't used by anything anyway.
https://review.coreboot.org/#/c/19206/6/util/git-hooks/pre-push File util/git-hooks/pre-push:
https://review.coreboot.org/#/c/19206/6/util/git-hooks/pre-push@46 PS6, Line 46: else : range="$remote_sha..$local_sha"
Nit, actually I don't like the `else` here, it's not directly
Yep, good point. Done.
-
David Hendricks (Code Review)