Re: [openstack-dev] Bad review patterns

2013-11-12 Thread Michael Davies
On Fri, Nov 8, 2013 at 4:07 AM, Pedro Roque Marques pedro.r.marq...@gmail.com wrote: Radomir, An extra issue that i don't believe you've covered so far is about comment ownership. I've just read an email on the list that follows a pattern that i've heard many complaints about: -1

Re: [openstack-dev] Bad review patterns

2013-11-11 Thread Joe Gordon
On Sun, Nov 10, 2013 at 3:50 PM, Sean Dague s...@dague.net wrote: Not that I know of. I've considered writing my own gerrit front end mail service to do just that, because I agree, the current mail volume and granularity is not very good. If I manage to carve time on it, I'll do it on

Re: [openstack-dev] Bad review patterns

2013-11-11 Thread Michael Basnight
On Nov 11, 2013, at 11:27 AM, Joe Gordon joe.gord...@gmail.com wrote: On Sun, Nov 10, 2013 at 3:50 PM, Sean Dague s...@dague.net wrote: Not that I know of. I've considered writing my own gerrit front end mail service to do just that, because I agree, the current mail volume and

Re: [openstack-dev] Bad review patterns

2013-11-11 Thread Mark McLoughlin
On Fri, 2013-11-08 at 09:32 +1300, Robert Collins wrote: On 7 November 2013 13:15, Day, Phil philip@hp.com wrote: Core reviewers look for the /comments/ from people, not just the votes. A +1 from someone that isn't core is meaningless unless they are known to be a thoughtful

Re: [openstack-dev] Bad review patterns

2013-11-11 Thread Mark McLoughlin
On Thu, 2013-11-07 at 20:40 -0500, David Ripton wrote: On 11/07/2013 07:54 PM, Sean Dague wrote: On 11/08/2013 01:37 AM, Pedro Roque Marques wrote: Radomir, An extra issue that i don't believe you've covered so far is about comment ownership. I've just read an email on the list that

Re: [openstack-dev] Bad review patterns

2013-11-09 Thread Sean Dague
Not that I know of. I've considered writing my own gerrit front end mail service to do just that, because I agree, the current mail volume and granularity is not very good. If I manage to carve time on it, I'll do it on stackforge. Joe Gordon took a different approach and wrote a front end client

Re: [openstack-dev] Bad review patterns

2013-11-07 Thread Radomir Dopieralski
On 06/11/13 23:07, Robert Collins wrote: On 6 November 2013 21:34, Radomir Dopieralski openst...@sheep.art.pl wrote: [...] Firstly, things like code duplication is a sliding scale, and I think it's ok for a reviewer to say 'these look similar, give it a go please'. If the reviewee tries, and

Re: [openstack-dev] Bad review patterns

2013-11-07 Thread Jiri Tomasek
On 11/07/2013 08:25 AM, Daniel P. Berrange wrote: On Thu, Nov 07, 2013 at 12:21:38AM +, Day, Phil wrote: Leaving a mark. === You review a change and see that it is mostly fine, but you feel that since you did so much work reviewing it, you should at least find *something*

Re: [openstack-dev] Bad review patterns

2013-11-07 Thread Robert Collins
On 7 November 2013 13:15, Day, Phil philip@hp.com wrote: Core reviewers look for the /comments/ from people, not just the votes. A +1 from someone that isn't core is meaningless unless they are known to be a thoughtful code reviewer. A -1 with no comment is also bad, because it doesn't

Re: [openstack-dev] Bad review patterns

2013-11-07 Thread Robert Collins
On 8 November 2013 00:02, Radomir Dopieralski openst...@sheep.art.pl wrote: I created a page on the wiki, https://wiki.openstack.org/wiki/CodeReviewGuidelines I put some initial content there, based on the discussion in this thread. Please feel free to discuss those points further here, and

Re: [openstack-dev] Bad review patterns

2013-11-07 Thread Sean Dague
On 11/07/2013 01:56 PM, James Bottomley wrote: On Thu, 2013-11-07 at 00:21 +, Day, Phil wrote: Leaving a mark. === You review a change and see that it is mostly fine, but you feel that since you did so much work reviewing it, you should at least find *something* wrong. So

Re: [openstack-dev] Bad review patterns

2013-11-07 Thread Sean Dague
On 11/07/2013 05:35 PM, Jiri Tomasek wrote: On 11/07/2013 08:25 AM, Daniel P. Berrange wrote: On Thu, Nov 07, 2013 at 12:21:38AM +, Day, Phil wrote: Leaving a mark. === You review a change and see that it is mostly fine, but you feel that since you did so much work

Re: [openstack-dev] Bad review patterns

2013-11-07 Thread Sean Dague
On 11/08/2013 01:37 AM, Pedro Roque Marques wrote: Radomir, An extra issue that i don't believe you've covered so far is about comment ownership. I've just read an email on the list that follows a pattern that i've heard many complaints about: -1 with a reasonable comment, submitter

Re: [openstack-dev] Bad review patterns

2013-11-07 Thread David Ripton
On 11/07/2013 07:54 PM, Sean Dague wrote: On 11/08/2013 01:37 AM, Pedro Roque Marques wrote: Radomir, An extra issue that i don't believe you've covered so far is about comment ownership. I've just read an email on the list that follows a pattern that i've heard many complaints about:

[openstack-dev] Bad review patterns

2013-11-06 Thread Radomir Dopieralski
Hello, I'm quite new in the OpenStack project, but I love it already. What is especially nifty is the automated review system -- I'm really impressed. I'm coming from a project in which we also did reviews of every change -- although it was mostly manual, and just one review was enough to merge

Re: [openstack-dev] Bad review patterns

2013-11-06 Thread Harshad Nakil
+1 Regards -Harshad On Nov 6, 2013, at 12:36 AM, Radomir Dopieralski openst...@sheep.art.pl wrote: Hello, I'm quite new in the OpenStack project, but I love it already. What is especially nifty is the automated review system -- I'm really impressed. I'm coming from a project in which

Re: [openstack-dev] Bad review patterns

2013-11-06 Thread Christopher Armstrong
On Wed, Nov 6, 2013 at 2:34 AM, Radomir Dopieralski openst...@sheep.art.plwrote: Hello, I'm quite new in the OpenStack project, but I love it already. What is especially nifty is the automated review system -- I'm really impressed. I'm coming from a project in which we also did reviews of

Re: [openstack-dev] Bad review patterns

2013-11-06 Thread Miguel Angel
All the points sound quite reasonable. I agree with Chris, the more reviewers read this, the better will be our review quality. Do we have some kind of reviewing guide?, if we don't this could be an start. -- irc: ajo / mangelajo Miguel Angel Ajo Pelayo +34 636 52 25 69 skype: ajoajoajo

Re: [openstack-dev] Bad review patterns

2013-11-06 Thread Sergey Skripnick
This definitely should be somewhere in wiki or blog and in the bookmarks. ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] Bad review patterns

2013-11-06 Thread Pitucha, Stanislaw Izaak
Skripnick [mailto:sskripn...@mirantis.com] Sent: Wednesday, November 06, 2013 6:50 PM To: OpenStack Development Mailing List (not for usage questions) Subject: Re: [openstack-dev] Bad review patterns This definitely should be somewhere in wiki or blog and in the bookmarks

Re: [openstack-dev] Bad review patterns

2013-11-06 Thread Andrew Woodward
: Wednesday, November 06, 2013 6:50 PM To: OpenStack Development Mailing List (not for usage questions) Subject: Re: [openstack-dev] Bad review patterns This definitely should be somewhere in wiki or blog and in the bookmarks. ___ OpenStack-dev mailing

Re: [openstack-dev] Bad review patterns

2013-11-06 Thread Ravi Chunduru
Pitucha Cloud Services Hewlett Packard -Original Message- From: Sergey Skripnick [mailto:sskripn...@mirantis.com] Sent: Wednesday, November 06, 2013 6:50 PM To: OpenStack Development Mailing List (not for usage questions) Subject: Re: [openstack-dev] Bad review patterns

Re: [openstack-dev] Bad review patterns

2013-11-06 Thread Eugene Nikanorov
Skripnick [mailto:sskripn...@mirantis.com] Sent: Wednesday, November 06, 2013 6:50 PM To: OpenStack Development Mailing List (not for usage questions) Subject: Re: [openstack-dev] Bad review patterns This definitely should be somewhere in wiki or blog and in the bookmarks

Re: [openstack-dev] Bad review patterns

2013-11-06 Thread Robert Collins
On 6 November 2013 21:34, Radomir Dopieralski openst...@sheep.art.pl wrote: Hello, I'm quite new in the OpenStack project, but I love it already. What is especially nifty is the automated review system -- I'm really impressed. I'm coming from a project in which we also did reviews of every

Re: [openstack-dev] Bad review patterns

2013-11-06 Thread Day, Phil
-Original Message- From: Robert Collins [mailto:robe...@robertcollins.net] Sent: 06 November 2013 22:08 To: OpenStack Development Mailing List (not for usage questions) Subject: Re: [openstack-dev] Bad review patterns On 6 November 2013 21:34, Radomir Dopieralski openst

Re: [openstack-dev] Bad review patterns

2013-11-06 Thread David Stanek
On Wed, Nov 6, 2013 at 7:21 PM, Day, Phil philip@hp.com wrote: Leaving a mark. === You review a change and see that it is mostly fine, but you feel that since you did so much work reviewing it, you should at least find *something* wrong. So you find some nitpick and

Re: [openstack-dev] Bad review patterns

2013-11-06 Thread James Bottomley
On Thu, 2013-11-07 at 00:21 +, Day, Phil wrote: Leaving a mark. === You review a change and see that it is mostly fine, but you feel that since you did so much work reviewing it, you should at least find *something* wrong. So you find some nitpick and -1 the

Re: [openstack-dev] Bad review patterns

2013-11-06 Thread Daniel P. Berrange
On Thu, Nov 07, 2013 at 12:21:38AM +, Day, Phil wrote: Leaving a mark. === You review a change and see that it is mostly fine, but you feel that since you did so much work reviewing it, you should at least find *something* wrong. So you find some nitpick and -1