Re: [Openstack] Code Reviews
+1 On Wed, May 11, 2011 at 2:38 PM, Vishvananda Ishaya vishvana...@gmail.comwrote: Hello Everyone, We have quite a large backlog of merge proposals here: https://code.launchpad.net/~rlane/nova/lp773690/+merge/59565 I've been attempting to go through them to find some high priority ones to review. It seems like people are being pretty active in reviewing branches, but there are a lot old branches that haven't been touched in a while. So first I have a general request that anyone who has old branches in for review: * please update your branches or mark them Work In Progress to remove them from the review queue.* I'd also like to propose a change to our process that will make the ready to review branches easier to find. I'd like for nova-core to set branches to WIP if there are two significant needs fixings or or needs information. That way everyone doesn't have to sort through branches that have already been reviewed but are waiting on updates. We may need to use our judgement here, so if a large branch has a needs fixing for a minor typo or some such, you could leave it under needs review so it gets viewed by more people. Here is an example where i think this policy will be useful: You see a branch that already has a 'Needs Fixing: this needs a failing test. If you look at the branch and reach the same conclusion, you can mark it Needs Fixing: I agree, needs a test like xxx and then set the branch to Work In Progress. When the author has added the test or needs to make more comments, he can set it back to Needs Review. I think this will generally keep the review board a little cleaner and also each branch will end up with a couple of people that are queued to review once the changes have come in. Does this seem acceptable to everyone? If I don't here any major dissents, I will add this info to the wiki and we can put it into practice. Vish ___ Mailing list: https://launchpad.net/~openstack Post to : openstack@lists.launchpad.net Unsubscribe : https://launchpad.net/~openstack More help : https://help.launchpad.net/ListHelp ___ Mailing list: https://launchpad.net/~openstack Post to : openstack@lists.launchpad.net Unsubscribe : https://launchpad.net/~openstack More help : https://help.launchpad.net/ListHelp
Re: [Openstack] Code Reviews
++ on your suggestions. On Wed, May 11, 2011 at 3:38 PM, Vishvananda Ishaya vishvana...@gmail.com wrote: Hello Everyone, We have quite a large backlog of merge proposals here: https://code.launchpad.net/~rlane/nova/lp773690/+merge/59565 I've been attempting to go through them to find some high priority ones to review. It seems like people are being pretty active in reviewing branches, but there are a lot old branches that haven't been touched in a while. So first I have a general request that anyone who has old branches in for review: please update your branches or mark them Work In Progress to remove them from the review queue. I'd also like to propose a change to our process that will make the ready to review branches easier to find. I'd like for nova-core to set branches to WIP if there are two significant needs fixings or or needs information. That way everyone doesn't have to sort through branches that have already been reviewed but are waiting on updates. We may need to use our judgement here, so if a large branch has a needs fixing for a minor typo or some such, you could leave it under needs review so it gets viewed by more people. Here is an example where i think this policy will be useful: You see a branch that already has a 'Needs Fixing: this needs a failing test. If you look at the branch and reach the same conclusion, you can mark it Needs Fixing: I agree, needs a test like xxx and then set the branch to Work In Progress. When the author has added the test or needs to make more comments, he can set it back to Needs Review. I think this will generally keep the review board a little cleaner and also each branch will end up with a couple of people that are queued to review once the changes have come in. Does this seem acceptable to everyone? If I don't here any major dissents, I will add this info to the wiki and we can put it into practice. Vish ___ Mailing list: https://launchpad.net/~openstack Post to : openstack@lists.launchpad.net Unsubscribe : https://launchpad.net/~openstack More help : https://help.launchpad.net/ListHelp ___ Mailing list: https://launchpad.net/~openstack Post to : openstack@lists.launchpad.net Unsubscribe : https://launchpad.net/~openstack More help : https://help.launchpad.net/ListHelp
Re: [Openstack] Code Reviews
I'm not nova-core, but this makes a heck of a lot of sense to me, too. On May 11, 2011, at 1:16 PM, Jay Pipes wrote: ++ on your suggestions. On Wed, May 11, 2011 at 3:38 PM, Vishvananda Ishaya vishvana...@gmail.com wrote: Hello Everyone, We have quite a large backlog of merge proposals here: https://code.launchpad.net/~rlane/nova/lp773690/+merge/59565 I've been attempting to go through them to find some high priority ones to review. It seems like people are being pretty active in reviewing branches, but there are a lot old branches that haven't been touched in a while. So first I have a general request that anyone who has old branches in for review: please update your branches or mark them Work In Progress to remove them from the review queue. I'd also like to propose a change to our process that will make the ready to review branches easier to find. I'd like for nova-core to set branches to WIP if there are two significant needs fixings or or needs information. That way everyone doesn't have to sort through branches that have already been reviewed but are waiting on updates. We may need to use our judgement here, so if a large branch has a needs fixing for a minor typo or some such, you could leave it under needs review so it gets viewed by more people. Here is an example where i think this policy will be useful: You see a branch that already has a 'Needs Fixing: this needs a failing test. If you look at the branch and reach the same conclusion, you can mark it Needs Fixing: I agree, needs a test like xxx and then set the branch to Work In Progress. When the author has added the test or needs to make more comments, he can set it back to Needs Review. I think this will generally keep the review board a little cleaner and also each branch will end up with a couple of people that are queued to review once the changes have come in. Does this seem acceptable to everyone? If I don't here any major dissents, I will add this info to the wiki and we can put it into practice. Vish ___ Mailing list: https://launchpad.net/~openstack Post to : openstack@lists.launchpad.net Unsubscribe : https://launchpad.net/~openstack More help : https://help.launchpad.net/ListHelp ___ Mailing list: https://launchpad.net/~openstack Post to : openstack@lists.launchpad.net Unsubscribe : https://launchpad.net/~openstack More help : https://help.launchpad.net/ListHelp ___ Mailing list: https://launchpad.net/~openstack Post to : openstack@lists.launchpad.net Unsubscribe : https://launchpad.net/~openstack More help : https://help.launchpad.net/ListHelp
Re: [Openstack] Code Reviews
2011/5/11 Vishvananda Ishaya vishvana...@gmail.com: I'd also like to propose a change to our process that will make the ready to [...] once the changes have come in. Does this seem acceptable to everyone? If I don't here any major dissents, I will add this info to the wiki and we can put it into practice. Sounds good to me. -- Soren Hansen | http://linux2go.dk/ Ubuntu Developer | http://www.ubuntu.com/ OpenStack Developer | http://www.openstack.org/ ___ Mailing list: https://launchpad.net/~openstack Post to : openstack@lists.launchpad.net Unsubscribe : https://launchpad.net/~openstack More help : https://help.launchpad.net/ListHelp
Re: [Openstack] Code Reviews
+1 On 5/11/11 3:16 PM, Jay Pipes jaypi...@gmail.com wrote: ++ on your suggestions. On Wed, May 11, 2011 at 3:38 PM, Vishvananda Ishaya vishvana...@gmail.com wrote: Hello Everyone, We have quite a large backlog of merge proposals here: https://code.launchpad.net/~rlane/nova/lp773690/+merge/59565 I've been attempting to go through them to find some high priority ones to review. It seems like people are being pretty active in reviewing branches, but there are a lot old branches that haven't been touched in a while. So first I have a general request that anyone who has old branches in for review: please update your branches or mark them Work In Progress to remove them from the review queue. I'd also like to propose a change to our process that will make the ready to review branches easier to find. I'd like for nova-core to set branches to WIP if there are two significant needs fixings or or needs information. That way everyone doesn't have to sort through branches that have already been reviewed but are waiting on updates. We may need to use our judgement here, so if a large branch has a needs fixing for a minor typo or some such, you could leave it under needs review so it gets viewed by more people. Here is an example where i think this policy will be useful: You see a branch that already has a 'Needs Fixing: this needs a failing test. If you look at the branch and reach the same conclusion, you can mark it Needs Fixing: I agree, needs a test like xxx and then set the branch to Work In Progress. When the author has added the test or needs to make more comments, he can set it back to Needs Review. I think this will generally keep the review board a little cleaner and also each branch will end up with a couple of people that are queued to review once the changes have come in. Does this seem acceptable to everyone? If I don't here any major dissents, I will add this info to the wiki and we can put it into practice. Vish ___ Mailing list: https://launchpad.net/~openstack Post to : openstack@lists.launchpad.net Unsubscribe : https://launchpad.net/~openstack More help : https://help.launchpad.net/ListHelp ___ Mailing list: https://launchpad.net/~openstack Post to : openstack@lists.launchpad.net Unsubscribe : https://launchpad.net/~openstack More help : https://help.launchpad.net/ListHelp Confidentiality Notice: This e-mail message (including any attached or embedded documents) is intended for the exclusive and confidential use of the individual or entity to which this message is addressed, and unless otherwise expressly indicated, is confidential and privileged information of Rackspace. Any dissemination, distribution or copying of the enclosed material is prohibited. If you receive this transmission in error, please notify us immediately by e-mail at ab...@rackspace.com, and delete the original message. Your cooperation is appreciated. ___ Mailing list: https://launchpad.net/~openstack Post to : openstack@lists.launchpad.net Unsubscribe : https://launchpad.net/~openstack More help : https://help.launchpad.net/ListHelp
Re: [Openstack] Code Reviews
+1 I just SmokeStack'ed a couple more suspiciously old branches and marked them needs fixing because they didn't merge w/ trunk. Most of which already had needs fixing for other reasons anyway. --- Also, I've been sitting on lp:~dan-prince/nova/chown_vdb_device for a month now. I'm happy to mark that as WIP to keep things clean. I am however okay with the merge prop as is (for the reasons in I listed in the merge prop). Dan -Original Message- From: Vishvananda Ishaya vishvana...@gmail.com Sent: Wednesday, May 11, 2011 3:38pm To: openstack@lists.launchpad.net Subject: [Openstack] Code Reviews ___ Mailing list: https://launchpad.net/~openstack Post to : openstack@lists.launchpad.net Unsubscribe : https://launchpad.net/~openstack More help : https://help.launchpad.net/ListHelp Hello Everyone, We have quite a large backlog of merge proposals here: https://code.launchpad.net/~rlane/nova/lp773690/+merge/59565 I've been attempting to go through them to find some high priority ones to review. It seems like people are being pretty active in reviewing branches, but there are a lot old branches that haven't been touched in a while. So first I have a general request that anyone who has old branches in for review: please update your branches or mark them Work In Progress to remove them from the review queue. I'd also like to propose a change to our process that will make the ready to review branches easier to find. I'd like for nova-core to set branches to WIP if there are two significant needs fixings or or needs information. That way everyone doesn't have to sort through branches that have already been reviewed but are waiting on updates. We may need to use our judgement here, so if a large branch has a needs fixing for a minor typo or some such, you could leave it under needs review so it gets viewed by more people. Here is an example where i think this policy will be useful: You see a branch that already has a 'Needs Fixing: this needs a failing test. If you look at the branch and reach the same conclusion, you can mark it Needs Fixing: I agree, needs a test like xxx and then set the branch to Work In Progress. When the author has added the test or needs to make more comments, he can set it back to Needs Review. I think this will generally keep the review board a little cleaner and also each branch will end up with a couple of people that are queued to review once the changes have come in. Does this seem acceptable to everyone? If I don't here any major dissents, I will add this info to the wiki and we can put it into practice. Vish ___ Mailing list: https://launchpad.net/~openstack Post to : openstack@lists.launchpad.net Unsubscribe : https://launchpad.net/~openstack More help : https://help.launchpad.net/ListHelp
