So I was looking at our git log. I can fit two, maybe three commits if I'm lucky. It's terrible. We have all this redundant information. We have both "Reviewed-on" and "Change-Id" lines. Those only point to the same resource. The "Tested-by" is useless. It's always Jenkins who tests changes. Then you have a "Reviewed-by" for every person who ever bikeshed on said patch. Since Paul has to say something on every single patch, he always has his own line in almost every commit.
"Reviewed-by" is not the same as "Acked-by". I've seen acked-by used to let linux folks know that some maintainer/big guy with more credentials than the author already likes the patch. Though Acked-by is not necessary, even for linux. Come to think of it, the only piece of information needed to find a commit on our gerrit is the hash. All the other lines, except for the "Signed- off-by" are redundant. "Reviewed-on" and "Change-Id" will take you to the same place as the commit hash. All the crap added by gerrit is redundant.
Now add a patch that comes from our Chromium friends, and you've doubled the size of said pork. Is it convenient to have all that information in the commit message? Maybe. Is it necessary? Definitely not. Those lines are useful for tracking a patch, but once it's +2'd and submitted, gerrit should strip out all that pork. We might not even use gerrit anymore a few years down the road. Adding all this crap is pointless.
It's time to drop the crap.
Alex
Ouch, sounds like you really had it with gerrit :)
While I agree that lots of information could be omitted and it would make more sense if only those who +1'ed or +2'ed a pach were mentioned, why do you care so mach?
And of course Change-ID is essential: the same patch can have different SHA1 because of rebases and might have different versions as a result of the reviews, and could be merged into multiple branches, etc,
The amount of email gerrit generates though could be easily cat to 25% and still be sufficient. There is a gerrit bug opened for that.
--vb
On Sun, Dec 28, 2014 at 8:32 PM, mrnuke mr.nuke.me@gmail.com wrote:
So I was looking at our git log. I can fit two, maybe three commits if I'm lucky. It's terrible. We have all this redundant information. We have both "Reviewed-on" and "Change-Id" lines. Those only point to the same resource. The "Tested-by" is useless. It's always Jenkins who tests changes. Then you have a "Reviewed-by" for every person who ever bikeshed on said patch. Since Paul has to say something on every single patch, he always has his own line in almost every commit.
"Reviewed-by" is not the same as "Acked-by". I've seen acked-by used to let linux folks know that some maintainer/big guy with more credentials than the author already likes the patch. Though Acked-by is not necessary, even for linux. Come to think of it, the only piece of information needed to find a commit on our gerrit is the hash. All the other lines, except for the "Signed- off-by" are redundant. "Reviewed-on" and "Change-Id" will take you to the same place as the commit hash. All the crap added by gerrit is redundant.
Now add a patch that comes from our Chromium friends, and you've doubled the size of said pork. Is it convenient to have all that information in the commit message? Maybe. Is it necessary? Definitely not. Those lines are useful for tracking a patch, but once it's +2'd and submitted, gerrit should strip out all that pork. We might not even use gerrit anymore a few years down the road. Adding all this crap is pointless.
It's time to drop the crap.
Alex
-- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot
-
mrnuke -
Vadim Bendebury