Re: [coreboot] [coreboot-gerrit] New patch to review for coreboot: 7d39656 lenovo/x60/romstage.c: Collect timestamps in romstage
ron minnich wrote:
Not really, those two lines are completely redundant.
The text is not needed, the link to gerrit is redundant, but the commit reference is nice to have. The commit message probably fits in about three lines.
//Peter
The message is much better, though. The reference to review.coreboot.org is a problem; it's a reference in a git message to something outside the repo. What if in 2 years gerrit is replaced by something else? We've got another bad URL, that's what. So let's stop dropping time bombs into commit messages. It's just a headache in the making.
So, best not to reference another commit at all, since the change is trivial anyway, but if you must, use the hash. No need to footnote, just put it inline, as every other project I'm involved in that uses git seems to do. Then we've got a hash in a log message, and it's all nicely self-contained.
But, again, no real need to reference another commit. This is a trivial change.
If we further remove the instructions for using timestamps and dumping them, which do not belong in the log, but on the wiki, we can reduce to the 3 lines Peter says it can be done in :-)
lenovo/x60/romstage.c: Collect timestamps in romstage
Collect early timestamps in Lenovo X60’s romstage.
Although I only see the need for one line, myself.
thanks
ron
On Thu, Jul 4, 2013 at 2:07 AM, Peter Stuge peter@stuge.se wrote:
-
Peter Stuge -
ron minnich