David Hendricks has posted comments on this change. ( https://review.coreboot.org/19206 )
Change subject: Convert flashrom to git ......................................................................
Patch Set 1:
(3 comments)
A few drive-by comments, I still need to dive into the getrivision.sh and Makefile changes.
https://review.coreboot.org/#/c/19206/1/util/git-hooks/install.sh File util/git-hooks/install.sh:
PS1, Line 17: ../../
This assumption looks rather odd given the `git rev-parse` above.
Yep, that appears to do the trick!
https://review.coreboot.org/#/c/19206/1/util/git-hooks/pre-push File util/git-hooks/pre-push:
PS1, Line 53: No Signoff or Ack
This reads abiguous. I think it wants to say "Either Signoff or Ack not fou
If we're going that route, "Neither Signoff nor Ack were found..." would sounds better IMO.
(http://dictionary.cambridge.org/us/grammar/british-grammar/questions-and-neg...)
PS1, Line 57: # Make _really_ sure we do not rewrite precious history : for lbranch in "${precious_branches[@]}"; do : if [ "$remote_ref" = "refs/heads/$lbranch" ]; then : nonreachable=$(git rev-list $remote_sha ^$local_sha) : if [ -n "$nonreachable" ]; then : echo "Only fast-forward pushes are allowed on $lbranch." >&2 : echo "$nonreachable is not included in $remote_sha while pusing to $remote_ref" >&2 : exit 1 : fi : fi : done
The code looks good, I was just worried that we reinvent the wheel.
There are other examples of people using this on stackoverflow so I guess it's generally accepted (e.g http://stackoverflow.com/a/15760683)
I guess we may as well just use receive.denyNonFastForwards until we come to a point where we want new branches?
-
David Hendricks (Code Review)