On Oct 16, 2013, at 15:16 , Sean Dague <s...@dague.net> wrote: > On 10/16/2013 01:19 AM, Alessandro Pilotti wrote: > <snip> >> >> Sean, you got "called out" in the meeting not because you asked to put a >> refernce link to the specs which was perfectly reasonable, but because >> after we did what you asked for in a timely manner, you didn't bother to >> review the patch again until asked to please review it 6 days later!!! >> >> This is a perfect example about why we need autonomy. We cannot leave a >> patch starving in the review queue for a critical bug like that one!! > > I -1ed the patch, you caught me on IRC and argued with me that the code > didn't need to change. You had my undivided attention there for 30 minutes on > this patch, but used the time to argue against change. So I moved on to other > things. Should I have gotten back around to my Nova review queue sooner, > sure. However once you made the fix I no longer had a -1 on the patch, so I > wasn't blocking it. And do I want to give up 30 minutes of my time every time > I try to review your patches because you'd rather argue than take feedback? > Not really. I still do it. But I'll admit, a patch author that gives me less > grief is a lot more fun to review. I'm only human in that regard. >
I beg for forgiveness for not obeying you on the spot and daring to discuss your -1, Master! ;-) Jokes aside, this is actually bringing up another important point in the review system: When somebody (especially a core reviewer) puts a -1 and a new patch is committed to address it, I noticed that other reviewers wait for the guy that put the -1 to say something before +1/+2 it. My feeling on this is that if somebody reviews a patch (positively or negatively) he/she should also keep on with it (in a timely manner) until it is merged or clearly stating that there's no interest in reviewing it further. This is especially true for core revs as other reviewers tend to be shy and avoid contradicting a core rev, generating further delays. What do you guys think? Does it make sense to brainstorm constructively on a way to reduce the review lags? The review system itself is IMO already providing an excellent starting point, we just need to tweak it a bit. :-) > 14 days from bug filing to merge isn't starving - > (https://bugs.launchpad.net/nova/+bug/1233853). If it's such a critical bug, > how come it didn't expose until 4 weeks after feature freeze? If it was such > a critical bug how did it get past your internal review process and land in > tree in the first place? If it's such a critical bug why wasn't it brought up > at the weekly *nova* meeting? > Because thanks to the gorgeus H3 phase we got all BPs merged together on the H3 freeze deadline and only afterwards people had the opportunity to test that huge amoung of code and report bugs? 14 days is IMO a preposterously long wait when you have a dedicated team and a fix ready, but hey, it's a matter of perspective I guess. > I really feel like you continue down the path of special pleading, without > having used normal channels for things like this, which all exist. The nova > meeting is a great place to highlight reviews you feel are critical that need > eyes, and it happens every week on a regular schedule. #OpenStack-Nova, ML and triaging bugs aren't normal channels? For how it is going I should bring every single bug and patch to the meeting! > > -Sean > > -- > Sean Dague > http://dague.net > > _______________________________________________ > OpenStack-dev mailing list > OpenStack-dev@lists.openstack.org > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev _______________________________________________ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
