We have also recently begun to do mostly-manditory code review for IPython. We are a much smaller project than Sage of course, but it is interesting that this (getting code reviews done promptly) is also a problem for us as well. We will watch anxiously to see what the magic solution is ;-)
Cheers, Brian On Mon, Dec 1, 2008 at 3:12 PM, mabshoff <[EMAIL PROTECTED]> wrote: > > > > On Dec 1, 12:07 pm, "Ondrej Certik" <[EMAIL PROTECTED]> wrote: >> On Mon, Dec 1, 2008 at 8:50 PM, Vinzent Steinberg >> >> <[EMAIL PROTECTED]> wrote: >> >> > Sage has problems with reviewing patches: >> >> >http://sagemath.blogspot.com/2008/11/sage-patch-review.html >> >> > I think our current patch review system could be improved too. So I >> > suggest to take part in the discussion. >> >> Indeed, thanks for pointing it out! > > For the record: I had a long discussion with William in #sage-devel > and then off-channel and in the end William did concede that his POV > was "overly simplistic" for some of the statements he made in that > thread. There is no other discussion about the issue in [sage-devel] > and I don't really see one happening since this issue was extensively > discussed at Dev1. Maybe the Thanksgiving weekend gave people other > priorities, so it might still happen. But I am not going to do a > public rebuttal to William's blog post since I don't want to spend the > time on writing up the document. The off-list discussion is also too > interlaced with the personal details of Sage developers that I won't > post it with everyone's permission. The main conclusion from Dev1 was > that something needed to be done, but having a multi person panel take > care of it didn't work out too well. I think the idea in itself wasn't > bad, its downfall was just that no one pushed hard for it or really > wanted to take charge. > > So in the end the situation can be summarized as "review is good, but > can be painful in certain situations". We all knew that when we > decided to introduce mandatory review and it has prevented a top of > crappy patches to be merged until they were fixed. The number of > patches in the Sage trac rise and fall exactly due to the effort > William made by pinging people personally and getting reviews done. > I.e. things build up until someone gets pissed and does something > about it. This has now happened twice (Dev1 saw a huge amount of > reviews due to the impeding merge of the new coercion system which > never happened in patch bomb form as anticipated prior to Dev1, the > other one was William now due to the upcoming ReST conversion of the > docstrings) and Sage Days 12 in San Diego in January will be 25+ Sage > Devs looked in a room for four days straight fixing bugs in trac so > that the number of open issues, especially the old ones, goes down > (consider on average two ticket per person per day resolved and we are > talking about 200 tickets resolved out of about 950 now :)) . It will > be great fun and the topic was suggested by me since I felt that too > many reported problem remain unfixed. > > A factoid: We merged 2900+ tickets since mandatory patch review was > made the default about a year ago, many of them with large patches or > patch series. When William wrote his blog post there were about 70 > unreviewed tickets in trac which was above average for sure. For 3.2.1 > we merged about 120 tickets in under 10 days, so those 70 tickets do > not represent a whole of a lot of development effort and drown out in > the turnover. What is a problem is that some tickets just sit there > and bitrot and that is why we need someone dedicated to taking care of > those tickets. That is all the conclusion that should be drawn out of > William's blog post, so painting it as a problem in the way William > did is "overly simplistic" for me :). > >> My own horse is gerrit, that Google uses, see this issue: >> >> http://code.google.com/p/sympy/issues/detail?id=1197 >> >> Andy tried to set it up, but it's a lot of work. >> >> Ondrej > > Cheers, > > Michael > > > --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "sympy" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/sympy?hl=en -~----------~----~----~----~------~----~------~--~---
