You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Hugo Trippaers <HT...@schubergphilis.com> on 2012/07/09 22:15:38 UTC

ReviewBoard

Heya all,

I was toying with the review board api and made a little report about our repo on ReviewBoard with some numbers.

Cheers,

Hugo

Reviewboard totals for cloudstack-git
Pending   :  67
Submitted :   5
Discarded :   3
Total     :  75

Name                  Pend  Subm  Disc
olga.smola              14     0     0
chip.childers           12     0     0
pranavs                  7     0     2
Spark404                 7     5     1
vijayendrabvs            5     0     0
jbausewein               4     0     0
deeptid                  3     0     0
likitha                  3     0     0
pradeepso                2     0     0
koushikd                 2     0     0
tsp                      2     0     0
anshulg                  1     0     0
jayapal                  1     0     0
rajesh_battala           1     0     0
mice_xia                 1     0     0
ke4qqq                   1     0     0
jlkinsel                 1     0     0

Pending review-requests without reviews or comments: (oldest first)
2012-06-21 11:56:41 pradeepso            : CS-15281: Removal of third party dependencies in Citrix code base.
2012-06-21 18:58:14 rajesh_battala       : Adding the replace.properties file back to master branch.  replace.properties file got deleted due to some commit in master branch. 
2012-06-27 11:35:18 pranavs              : CS-15211 - At the end of the wizard if there are errors you are provided with links back to the step where it thinks the error occurred. It just wasn't clear that you could get out of the step that the wizard linked you to and move to a different step where the error actually originated. 
2012-06-28 00:18:48 vijayendrabvs        : Review request for bug 15372.
2012-06-29 09:00:59 olga.smola           : CS-15391: Missing required fields velidation when Edit on the Details tab for Infrastructure/Zones, Infrastructure/Pods.
2012-06-29 09:32:21 olga.smola           : CS-15392: Incorrect results when edit user.
2012-06-29 10:47:23 olga.smola           : CS-15393: Scroll bar doesn't work when any point is selected.
2012-07-02 10:33:49 tsp                  : Appending/Cleaning up the ASF license headers for Marvin
2012-07-03 14:34:31 olga.smola           : CS-15353: UI Larger click boxes in UI
2012-07-03 15:32:02 koushikd             : Fix for CS-15279
2012-07-04 05:40:17 mice_xia             : fix CS-15432 Failed to detach VMware tools ISO after VMware tools installation
2012-07-04 06:29:38 koushikd             : Fix for CS-15345
2012-07-06 08:39:54 Spark404             : PremiumSecondaryStorageResource should be part of core and not of the vmware plugin
2012-07-06 11:08:41 likitha              : Implementation of 3 ec2 commands: ec2-create-tags, ec2-delete-tags and ec2-decribe-tags. Component: AWSAPI.
2012-07-06 11:47:40 Spark404             : cloud-vmware.jar needs to be in the system vm for the PremiumSecondaryStorageResource to work
2012-07-06 15:37:07 olga.smola           : CS-15478: UI cosmetic bug, empty buttons/viewAll section.
2012-07-09 06:35:49 Spark404             : parms should be a copy instead of a reference as locals() is bound to change over time
2012-07-09 09:04:27 Spark404             : Typo in components.xml.in
2012-07-09 12:41:54 Spark404             : fix for broken attachVolume command
2012-07-09 18:38:57 Spark404             : This table should not have a contraint on device and physical network id. This is dealt with in the code and by using the removed field.
2012-07-09 19:42:58 jbausewein           : Hiding of add guest network button

RE: ReviewBoard

Posted by Sudha Ponnaganti <su...@citrix.com>.
Also besides review metrics, is there any way to log and track defects (within Review Board) during code review like  CodeCollaborator. 

-----Original Message-----
From: David Nalley [mailto:david@gnsa.us] 
Sent: Monday, July 09, 2012 7:12 PM
To: cloudstack-dev@incubator.apache.org
Subject: Re: ReviewBoard

On Mon, Jul 9, 2012 at 8:05 PM, Kevin Kluge <Ke...@citrix.com> wrote:
> I'm surprised by the number of Pending and Submitted.  I have seen so many approval messages (I counted 48 in my quick mailbox search) that it seems odd that only 5 patches are classified as submitted.  Are people just not completing the workflow when something gets submitted?
>
> -kevin
>

So I have found that if I use the exact phrase and author in the commit it will close it for me.
Otherwise, it will look like it is just hanging out there forever.
There is supposed to be an option to close - but it doesn't appear in the UI (and doesn't appear to have the options shown in the manual - so I suspect there is a perms issue). So in short, yes a workflow issue.

--David

Re: ReviewBoard

Posted by David Nalley <da...@gnsa.us>.
On Wed, Jul 11, 2012 at 4:16 PM, Kevin Kluge <Ke...@citrix.com> wrote:
> Are we agreed this is the workflow -- original submitter should check and then close as Submitted after the checkin?
>
> We should also have the committers " use the exact phrase and author in the commit" as David says.    This seems preferable as is effectively automated.
>
> -kevin

Hugo's emails suggest that what I was seeing might not be the way it
was working, and that he was manually making that happen.

--David

RE: ReviewBoard

Posted by Kevin Kluge <Ke...@citrix.com>.
Are we agreed this is the workflow -- original submitter should check and then close as Submitted after the checkin?

We should also have the committers " use the exact phrase and author in the commit" as David says.    This seems preferable as is effectively automated.

-kevin

> -----Original Message-----
> From: Olga Smola [mailto:olya.smola@gmail.com]
> Sent: Wednesday, July 11, 2012 8:21 AM
> To: cloudstack-dev@incubator.apache.org
> Subject: Re: ReviewBoard
> 
> David,
> 
> thank you. Ok, every ticket should be checked if it really was committed.
> 
> Olga
> 
> On Wed, Jul 11, 2012 at 6:14 PM, David Nalley <da...@gnsa.us> wrote:
> 
> > On Wed, Jul 11, 2012 at 11:08 AM, Olga Smola <ol...@gmail.com>
> wrote:
> > > So if request gets "Ship it!", it should be submitted (choose the
> > reviewed
> > > ticket and from the menu "Close"/"Submitted"), am I right?
> > >
> > > Olga
> >
> > Correct - though I'd confirm that there is actually a corresponding
> > commit that is actually made. There have been a number that have had
> > 'ship it' in the review that didn't actually get committed til several
> > days later.
> >
> > --David
> >

Re: ReviewBoard

Posted by Olga Smola <ol...@gmail.com>.
David,

thank you. Ok, every ticket should be checked if it really was committed.

Olga

On Wed, Jul 11, 2012 at 6:14 PM, David Nalley <da...@gnsa.us> wrote:

> On Wed, Jul 11, 2012 at 11:08 AM, Olga Smola <ol...@gmail.com> wrote:
> > So if request gets "Ship it!", it should be submitted (choose the
> reviewed
> > ticket and from the menu "Close"/"Submitted"), am I right?
> >
> > Olga
>
> Correct - though I'd confirm that there is actually a corresponding
> commit that is actually made. There have been a number that have had
> 'ship it' in the review that didn't actually get committed til several
> days later.
>
> --David
>

Re: ReviewBoard

Posted by David Nalley <da...@gnsa.us>.
On Wed, Jul 11, 2012 at 11:08 AM, Olga Smola <ol...@gmail.com> wrote:
> So if request gets "Ship it!", it should be submitted (choose the reviewed
> ticket and from the menu "Close"/"Submitted"), am I right?
>
> Olga

Correct - though I'd confirm that there is actually a corresponding
commit that is actually made. There have been a number that have had
'ship it' in the review that didn't actually get committed til several
days later.

--David

Re: ReviewBoard

Posted by Olga Smola <ol...@gmail.com>.
So if request gets "Ship it!", it should be submitted (choose the reviewed
ticket and from the menu "Close"/"Submitted"), am I right?

Olga

On Tue, Jul 10, 2012 at 9:29 AM, Hugo Trippaers <
HTrippaers@schubergphilis.com> wrote:

> I think opnly the orginal submitter can close the request by selecting
> submitted or discarded. Indeed a workflow issue, I tend to close a request
> when it is applied by a committer, maybe it's nice to remind folks that
> they can close the request after it is applied. Maybe somebody can get
> permissions to also close requests by asking Infra team?
>
> Hugo
>
> -----Original Message-----
> From: David Nalley [mailto:david@gnsa.us]
> Sent: Tuesday, July 10, 2012 4:12 AM
> To: cloudstack-dev@incubator.apache.org
> Subject: Re: ReviewBoard
>
> On Mon, Jul 9, 2012 at 8:05 PM, Kevin Kluge <Ke...@citrix.com>
> wrote:
> > I'm surprised by the number of Pending and Submitted.  I have seen so
> many approval messages (I counted 48 in my quick mailbox search) that it
> seems odd that only 5 patches are classified as submitted.  Are people just
> not completing the workflow when something gets submitted?
> >
> > -kevin
> >
>
> So I have found that if I use the exact phrase and author in the commit it
> will close it for me.
> Otherwise, it will look like it is just hanging out there forever.
> There is supposed to be an option to close - but it doesn't appear in the
> UI (and doesn't appear to have the options shown in the manual - so I
> suspect there is a perms issue). So in short, yes a workflow issue.
>
> --David
>

RE: ReviewBoard

Posted by Hugo Trippaers <HT...@schubergphilis.com>.
I think opnly the orginal submitter can close the request by selecting submitted or discarded. Indeed a workflow issue, I tend to close a request when it is applied by a committer, maybe it's nice to remind folks that they can close the request after it is applied. Maybe somebody can get permissions to also close requests by asking Infra team?

Hugo

-----Original Message-----
From: David Nalley [mailto:david@gnsa.us] 
Sent: Tuesday, July 10, 2012 4:12 AM
To: cloudstack-dev@incubator.apache.org
Subject: Re: ReviewBoard

On Mon, Jul 9, 2012 at 8:05 PM, Kevin Kluge <Ke...@citrix.com> wrote:
> I'm surprised by the number of Pending and Submitted.  I have seen so many approval messages (I counted 48 in my quick mailbox search) that it seems odd that only 5 patches are classified as submitted.  Are people just not completing the workflow when something gets submitted?
>
> -kevin
>

So I have found that if I use the exact phrase and author in the commit it will close it for me.
Otherwise, it will look like it is just hanging out there forever.
There is supposed to be an option to close - but it doesn't appear in the UI (and doesn't appear to have the options shown in the manual - so I suspect there is a perms issue). So in short, yes a workflow issue.

--David

Re: ReviewBoard

Posted by David Nalley <da...@gnsa.us>.
On Mon, Jul 9, 2012 at 8:05 PM, Kevin Kluge <Ke...@citrix.com> wrote:
> I'm surprised by the number of Pending and Submitted.  I have seen so many approval messages (I counted 48 in my quick mailbox search) that it seems odd that only 5 patches are classified as submitted.  Are people just not completing the workflow when something gets submitted?
>
> -kevin
>

So I have found that if I use the exact phrase and author in the
commit it will close it for me.
Otherwise, it will look like it is just hanging out there forever.
There is supposed to be an option to close - but it doesn't appear in
the UI (and doesn't appear to have the options shown in the manual -
so I suspect there is a perms issue). So in short, yes a workflow
issue.

--David

RE: ReviewBoard

Posted by Kevin Kluge <Ke...@citrix.com>.
I'm surprised by the number of Pending and Submitted.  I have seen so many approval messages (I counted 48 in my quick mailbox search) that it seems odd that only 5 patches are classified as submitted.  Are people just not completing the workflow when something gets submitted?

-kevin


> -----Original Message-----
> From: Hugo Trippaers [mailto:HTrippaers@schubergphilis.com]
> Sent: Monday, July 09, 2012 1:16 PM
> To: cloudstack-dev@incubator.apache.org
> Subject: ReviewBoard
> 
> Heya all,
> 
> I was toying with the review board api and made a little report about our
> repo on ReviewBoard with some numbers.
> 
> Cheers,
> 
> Hugo
> 
> Reviewboard totals for cloudstack-git
> Pending   :  67
> Submitted :   5
> Discarded :   3
> Total     :  75
> 
> Name                  Pend  Subm  Disc
> olga.smola              14     0     0
> chip.childers           12     0     0
> pranavs                  7     0     2
> Spark404                 7     5     1
> vijayendrabvs            5     0     0
> jbausewein               4     0     0
> deeptid                  3     0     0
> likitha                  3     0     0
> pradeepso                2     0     0
> koushikd                 2     0     0
> tsp                      2     0     0
> anshulg                  1     0     0
> jayapal                  1     0     0
> rajesh_battala           1     0     0
> mice_xia                 1     0     0
> ke4qqq                   1     0     0
> jlkinsel                 1     0     0
> 
> Pending review-requests without reviews or comments: (oldest first)
> 2012-06-21 11:56:41 pradeepso            : CS-15281: Removal of third party
> dependencies in Citrix code base.
> 2012-06-21 18:58:14 rajesh_battala       : Adding the replace.properties file
> back to master branch.  replace.properties file got deleted due to some
> commit in master branch.
> 2012-06-27 11:35:18 pranavs              : CS-15211 - At the end of the wizard if
> there are errors you are provided with links back to the step where it thinks
> the error occurred. It just wasn't clear that you could get out of the step that
> the wizard linked you to and move to a different step where the error
> actually originated.
> 2012-06-28 00:18:48 vijayendrabvs        : Review request for bug 15372.
> 2012-06-29 09:00:59 olga.smola           : CS-15391: Missing required fields
> velidation when Edit on the Details tab for Infrastructure/Zones,
> Infrastructure/Pods.
> 2012-06-29 09:32:21 olga.smola           : CS-15392: Incorrect results when edit
> user.
> 2012-06-29 10:47:23 olga.smola           : CS-15393: Scroll bar doesn't work when
> any point is selected.
> 2012-07-02 10:33:49 tsp                  : Appending/Cleaning up the ASF license
> headers for Marvin
> 2012-07-03 14:34:31 olga.smola           : CS-15353: UI Larger click boxes in UI
> 2012-07-03 15:32:02 koushikd             : Fix for CS-15279
> 2012-07-04 05:40:17 mice_xia             : fix CS-15432 Failed to detach VMware
> tools ISO after VMware tools installation
> 2012-07-04 06:29:38 koushikd             : Fix for CS-15345
> 2012-07-06 08:39:54 Spark404             : PremiumSecondaryStorageResource
> should be part of core and not of the vmware plugin
> 2012-07-06 11:08:41 likitha              : Implementation of 3 ec2 commands: ec2-
> create-tags, ec2-delete-tags and ec2-decribe-tags. Component: AWSAPI.
> 2012-07-06 11:47:40 Spark404             : cloud-vmware.jar needs to be in the
> system vm for the PremiumSecondaryStorageResource to work
> 2012-07-06 15:37:07 olga.smola           : CS-15478: UI cosmetic bug, empty
> buttons/viewAll section.
> 2012-07-09 06:35:49 Spark404             : parms should be a copy instead of a
> reference as locals() is bound to change over time
> 2012-07-09 09:04:27 Spark404             : Typo in components.xml.in
> 2012-07-09 12:41:54 Spark404             : fix for broken attachVolume command
> 2012-07-09 18:38:57 Spark404             : This table should not have a contraint on
> device and physical network id. This is dealt with in the code and by using the
> removed field.
> 2012-07-09 19:42:58 jbausewein           : Hiding of add guest network button

Re: ReviewBoard

Posted by David Nalley <da...@gnsa.us>.
On Mon, Jul 9, 2012 at 4:15 PM, Hugo Trippaers
<HT...@schubergphilis.com> wrote:
> Heya all,
>
> I was toying with the review board api and made a little report about our repo on ReviewBoard with some numbers.
>
> Cheers,
>

First of all - this is awesome - would you mind submitting a patch
with the code you are running to do this and put it somewhere under
project_admin/ ?
Maybe we can cron it up and have it submit a report weekly.

--David