From: John Keeping <john@keeping.me.uk>
To: Michael Haggerty <mhagger@alum.mit.edu>
Cc: Junio C Hamano <gitster@pobox.com>,
Ramkumar Ramachandra <artagnon@gmail.com>,
Git List <git@vger.kernel.org>,
Jonathan Nieder <jrnieder@gmail.com>,
A Large Angry SCM <gitzilla@gmail.com>
Subject: Re: [PATCH] Documentation/CommunityGuidelines
Date: Tue, 11 Jun 2013 20:19:36 +0100 [thread overview]
Message-ID: <20130611191936.GN22905@serenity.lan> (raw)
In-Reply-To: <51B771D5.6030809@alum.mit.edu>
On Tue, Jun 11, 2013 at 08:52:05PM +0200, Michael Haggerty wrote:
> That's a very good point (and a good illustration, too). How do you
> like the new second and third sentences below?
>
> * When reviewing other peoples' code, be tactful and constructive.
> Remember that submitting patches for public critique can be very
> intimidating and when mistakes are found it can be embarrassing. Do
> what you can to make it a positive and pleasant experience for the
> submitter. Set high expectations, but do what you can to help the
> submitter achieve them. Don't demand changes based only on your
> personal preferences. Don't let the perfect be the enemy of the good.
I'm not sure. I like the intent, but I'm not sure that it's clear
enough that we're talking about the tone of comments rather than the
type of feedback to provide.
How about something like this?
* Having your code reviewed should feel like a collaboration aiming
for the best result for the project, not like a fight to get your
patch accepted. Try to bear this in mind when reviewing other
peoples' code and consider how you would feel reading the same
comments if the review was the other way round. We are only human
and the tone of a review can influence how the following
discussion progresses.
* If you do feel that a review is aggressive, don't reply
immediately. Contributors are spread around the world in
different timezones and it is often better to wait a few hours for
others to comment before rushing to defend your patch.
next prev parent reply other threads:[~2013-06-11 19:19 UTC|newest]
Thread overview: 63+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-06-10 13:28 [PATCH] Documentation/CommunityGuidelines Ramkumar Ramachandra
2013-06-10 13:50 ` Célestin Matte
2013-06-10 14:04 ` Matthieu Moy
2013-06-10 16:25 ` Robin H. Johnson
2013-06-10 17:42 ` Junio C Hamano
2013-06-10 19:01 ` Jonathan Nieder
2013-06-10 19:45 ` Ramkumar Ramachandra
2013-06-10 20:41 ` A Large Angry SCM
2013-06-10 20:56 ` Ramkumar Ramachandra
2013-06-10 21:09 ` A Large Angry SCM
2013-06-11 5:16 ` Felipe Contreras
2013-06-11 8:56 ` Ramkumar Ramachandra
2013-06-11 4:41 ` Michael Haggerty
2013-06-11 6:28 ` Felipe Contreras
2013-06-11 10:45 ` Ramkumar Ramachandra
2013-06-11 11:49 ` Felipe Contreras
2013-06-11 12:33 ` Thomas Rast
2013-06-11 13:40 ` Ramkumar Ramachandra
2013-06-11 14:40 ` Michael Haggerty
2013-06-11 15:34 ` Felipe Contreras
2013-06-11 18:16 ` Ramkumar Ramachandra
2013-06-11 18:43 ` Michael Haggerty
2013-06-11 18:55 ` Ramkumar Ramachandra
2013-06-11 19:06 ` Junio C Hamano
2013-06-11 19:39 ` Felipe Contreras
2013-06-11 23:48 ` Felipe Contreras
2013-06-11 19:55 ` Brandon Casey
2013-06-12 11:56 ` Theodore Ts'o
2013-06-12 12:29 ` Ramkumar Ramachandra
2013-06-12 13:58 ` Felipe Contreras
2013-06-11 15:06 ` Felipe Contreras
2013-06-11 15:41 ` Thomas Rast
2013-06-11 15:52 ` Felipe Contreras
2013-06-11 16:10 ` Thomas Rast
2013-06-11 16:17 ` Felipe Contreras
2013-06-11 17:00 ` Junio C Hamano
2013-06-11 18:24 ` Michael Haggerty
2013-06-11 18:29 ` John Keeping
2013-06-11 18:46 ` Ramkumar Ramachandra
2013-06-11 19:54 ` John Keeping
2013-06-12 11:26 ` Ramkumar Ramachandra
2013-06-12 13:14 ` John Keeping
2013-06-11 18:52 ` Michael Haggerty
2013-06-11 19:19 ` John Keeping [this message]
2013-06-11 19:46 ` Philip Oakley
2013-06-12 0:08 ` John Szakmeister
2013-06-12 14:49 ` Jakub Narebski
2013-06-12 20:54 ` Philip Oakley
2013-06-11 19:35 ` Felipe Contreras
2013-06-11 20:33 ` Jeff King
2013-06-11 20:55 ` Junio C Hamano
2013-06-11 23:19 ` Felipe Contreras
2013-06-12 12:27 ` Theodore Ts'o
2013-06-12 14:06 ` Felipe Contreras
2013-06-12 12:03 ` Ramkumar Ramachandra
2013-06-12 20:02 ` Junio C Hamano
2013-06-13 3:45 ` Michael Haggerty
2013-06-13 4:22 ` Junio C Hamano
2013-06-11 5:38 ` Felipe Contreras
2013-06-11 11:11 ` Ramkumar Ramachandra
2013-06-13 10:19 ` Thomas Adam
2013-06-13 13:36 ` Felipe Contreras
2013-06-14 9:48 ` Christian Couder
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130611191936.GN22905@serenity.lan \
--to=john@keeping.me.uk \
--cc=artagnon@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=gitzilla@gmail.com \
--cc=jrnieder@gmail.com \
--cc=mhagger@alum.mit.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).