[haiku-commits] Re: r40520 - in haiku/trunk/src: add-ons/kernel/drivers/graphics/radeon_hd tests/kits/net/preflet/InterfacesAddOn

  • From: Simon Taylor <simontaylor1@xxxxxxxxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Thu, 17 Feb 2011 08:59:09 +0000

Axel Dörfler wrote:
Jonas Sundström<jonasjavascript:msgAction('send');@kirilla.com> wrote:
Jérôme Duval Jérôme Duval <korli@xxxxxxxxxxxxxxxx> wrote:
I guess you forgot the time involved in reviewing and pointing such
mistakes.

Indeed, however...

I understand the necessity of code review, and that giving
feedback is often a thankless task.

I'm embarrased to say that I don't review commits. So I am not
in a position to argue that the reviews are unfair, or that they
should be some other way. For having done so, I apologize.

... I think it's perfectly okay to point out things one finds unfair, even if you might 
not have taken the full picture into account. I know I am sometimes a bit too 
"direct" in my criticism, and mentioning that from time to time certainly 
doesn't harm.

Group hug? ;)

Never having done it myself it's hard to know how frustrated I would get by taking the time to review and then have the same errors crop up again.

However I wonder whether it was a bit more "direct" than usual this time due to the very quick way that Alex was granted commit access - there was no official vote, and only replies from Clemens, Stephan, Jerome and Philippe before the change was made (Matt did wait 2 days after the final reply before making the change though). There was also no "I'll accept in two days unless there's any objections" note.

Perhaps as Alex's network preference work was obviously going to be touching Axel's network code the final decision should have waited until Axel had made his opinion known? Having one's acceptance assumed might lead to a somewhat (possibly subconscious) overbearing attitude to the first commits of the newbie, which should ideally be a supportive time as they adjust to the expected standards.

Simon
(very) amateur psychologist

Other related posts: