On Sun, Aug 15, 2010 at 3:03 PM, burc...@gmail.com <burc...@gmail.com> wrote: > Hi Russell, > > On Sat, Aug 14, 2010 at 2:03 PM, Russell Keith-Magee > <russ...@keith-magee.com> wrote: >> On Sat, Aug 14, 2010 at 1:39 PM, burc...@gmail.com <burc...@gmail.com> wrote: >>> I had few useful thoughts about changing the way Django development >>> contributions gets accepted and committed -- but all I get from this >>> mailing list that all attempts to improve any process is just a waste >>> of time. >> >> That's not entirely fair. >> >> We're acutely aware of the fact that there is a backlog of tickets >> that need attention. We're also aware that many people upload patches, >> but then don't get any feedback on them. We want to improve this in >> any way we can. >> >> If you have suggestions on how we can improve Django's development >> process and address these issues, we're happy to hear them. >> >> However, that doesn't mean we're going to immediately implement a >> suggestion just because it's been made. I don't know which suggestion >> in particular you're referring to, but if you've made a suggestion and >> we haven't acted on it, then I would suggest to you that this is >> because we (the core team) didn't agree that your idea was as useful >> or practical as you think it was. >> >> If you have a suggestion, please make it. We're listening. > > Ok. I think the reviewing workflow should be improved in the way core > developers, triagers and patch developers should get a list of tickets > to work on. Please correct me if I'm wrong, but I don't see any > mechanism that will allow you: > a) to find issues that needs a review (not some issues, but all issues > ordered by importance or date sent to reviewing)
http://code.djangoproject.com/query?status=new&status=assigned&status=reopened&needs_better_patch=%211&has_patch=1&order=priority&stage=Accepted That is -- all the tickets that have been accepted and have a patch that is currently marked as "ok". There's a separate query that also requires attention: http://code.djangoproject.com/query?status=new&status=assigned&status=reopened&order=priority&stage=Unreviewed This is the triage task, and it's just as important as writing/reviewing patches. Ordering by 'importance' isn't something we support. Historically, we did, and it consistently proved to be almost useless as a measure of anything. Everyone thinks that the problem they submit is very important, so having importance as an open field in Trac doesn't yield useful data. A more helpful measure of 'importance' is the vitality of the discussion under the ticket; a ticket with lots of interest will almost always have a large number of comments, usually a couple of which that say "why hasn't this been merged yet?". The other useful measure is the number of times an issue is re-reported. We don't have a good measure of this, AFAIK. Ordering by date isn't something that we expose particularly well either, but we have an analog -- the ticket number. Tickets are allocated sequentially, so ticket 1234 was logged before 2345. It doesn't take too long working with Trac to work out that a ticket with a number in the 8000 range is about 2 years old; 14000 is less than a month old. > b) to mark issue as reviewed. This one we definitely support. Tickets that have been reviewed and are good: http://code.djangoproject.com/query?status=new&status=assigned&status=reopened&order=priority&stage=Ready+for+checkin Tickets with a negative review: http://code.djangoproject.com/query?status=new&status=assigned&status=reopened&needs_better_patch=1&order=priority > c) to mark someone (core dev/someone else) as a curator on an issue, > so core team doesn't repeat themselves reviewing each bug multiple > times when it's not required. > The problem issues don't get attention is that no one usually know > they need their attention. > Bug states don't help a lot: they mark the maturity of solution but > not who's move is now. > And current process from my point of view is a Monte-Carlo issue > picking. It's not broken -- speed is almost the same (cause it's the > speed of commit process), but absolutely no feeling of control. I'm not sure exactly what you're concerned about here. The "curator" of an issue is the owner. If you claim a ticket, you are curating it. As for the problem of multiple reviewing: Allow me to assure you that for me personally, 'double reviewing' of tickets isn't a problem I face. I have three working lists for tickets: * The list of tickets marked against milestone 1.3 * The list of tickets marked Ready For Checkin * My personal list of tickets that I've flagged as interesting. This usually means that something in the report caught my attention, or because someone on django-dev has made a polite request for review. If I review a ticket on the RFC list and it isn't RFC, it gets bumped back to Accepted (with any other flag changes that are required) If a ticket is on either of the other two working lists, I work on them until they're finished (which means the ticket is either committed, or closed wontfix/invalid etc). That's because these lists are driven by a different set of priorities; Milestone lists because they're standing between me and a RC; personal lists are things that are my own itches that I want to scratch. It shouldn't be a problem anyone else faces, either. The "problem issues that need attention" is "any ticket that isn't currently closed". The "who's move is now" can be determined by reading the ticket; it's either the owner of the ticket (or the person who has provided the patch), or the ticket needs review -- from anyone who can volunteer the time. If the issue is that this is an imposing list, that's because there's lots of work to do. Adding extra triage states doesn't reduce the amount of outstanding work -- it just diverts time that could be spent fixing bugs into bit twiddling on trac. If the number of tickets requiring work is imposing, I would suggest that the 'component' flag is the best way to combat this. This allows you to focus on a particular area (e.g., contrib.auth), which provides focus on the tasks to be done, and reduces the overwhelming nature of the ticket list. It also helps me -- when I sit down to commit, I vastly prefer concentrating my efforts in a single area of code, since it allows me to get familiar with a single area of code and fix multiple issues in rapid succession. > Also after the review/commit separation, we'll be able to tell > exactly, if we need more reviewers, patch writers/rewriters, core dev > team members or just committers. I don't need any changes to Trac to answer this question. The answer to this is "Yes". :-) >>> Core devs have no time even to accept working & looking good >>> patches -- and rarely have time to review patches not looking good or >>> working wrong! >>> So why should I bother to write patches fast? >> >> I would point out that the core team aren't the only people that can >> give feedback. Trac is an open resource for exactly that reason. > > Yes, this is one more point. Regular developers, like me, also don't > know what to review, and for most of patches, they just can't write a > review: they have high risk of writing wrong suggestion. Patch author > or triagers usually know if people can write a review, but people > can't find such issues. There's no nice way to sugar coat this. There is a lot of work that needs to be done. We have a lot of open tickets. The most important thing is to do *something*. Be bold -- if you review something and you think you've go the right solution, mark it as Ready For Checkin. If you want clarification on something before you do this, ask on django-dev (mailing list or IRC). Yes, some of your reviews will be wrong, especially in your first attempts. However, allow me to assure you that if I find a patch in Ready For Checkin that doesn't meet trunk standards, I'll push it back to Accepted with a 'patch needs improvement flag', along with review notes. > So probably "reviewers" bugs category should also exist, but should be > also treated differently. I don't understand what you're proposing here. Again, any ticket that isn't in RFC needs either a review or a patch. Trac already gives you the tools to separate those two lists. > If we agree with such a system, I can walk through all issues > (probably with scripts), and write who needs to respond on each issue. > Patch developers usually know whether everyone wants feedback from > them, but I'd love if we were able to reduce core developers load with > introduction of such system and adding more reviewers. This is where your argument falls apart. Adding extra flags in Trac doesn't increase the number of reviewers that we have. Sending an email to someone doesn't compel them to do work -- this is a volunteer project. If anything, bit twiddling on Trac *reduces* the amount of review work that gets done, because the effort is being spent twiddling bits rather than reviewing actual code. What adds more reviewers is people actually doing work. I'm afraid I don't see how increasing the complexity of the interaction in Trac serves to increase the number of people contributing. However, I will concede that Trac has a fairly awful UI, and there may be room for improvement to polish our Trac install to make it easier to consume Trac information (for example, having more prominent links to useful Trac query reports). If Trac configuration is an area in which you have expertise, we'd be happy for the help. (... and to head the inevitable argument off at the pass... that isn't an invitation to propose adopting an alternatives to Trac. *All* bug trackers suck, they just suck in different ways. Trac sucks in ways we understand.) > Think of the following: every personal django branch on github can be > considered as adding a reviewer & committer. Why not ask them to help > with tickets? But then they need to know how they can help. We outline > issues to write patches for, but not for review. You appear to be under the impression that we haven't done this. I've lost count of the number of times I've asked people to contribute. I've also lost count of the number of times I've told people that if they provide a git/hg branch that is a rich vein of trunk-ready fixes, I'll merge that branch, and if they make a habit of providing trunk-ready patches, we can see about getting trunk commit access. > Since tickets with needs_better_patch=1 could be both reviewed or not > reviewed, I haven't find any way to get any reviewing queue. Agreed, there is a slight ambiguity in the 'needs-better-patch' flag. The obvious fix is to make it a tri-state flag instead of a boolean -- * Needs Review * Patch needs improvement * Patch is OK. However, this ignores the overlap with 'ready for checkin'. A ticket that is 'ready for checkin' tells you that the patch is ok. If you have a patch that is 'needs-better-patch=false', but it isn't in 'ready for checkin', then it hasn't been reviewed. I will completely agree that this isn't ideal from a UX point of view (i.e., it's logically consistent if you think about it, but the fact you need to think about it isn't a good thing). However, we're slightly hamstrung by what Trac can give us here. > On implementation side, I consider two checkboxes with "[ ] needs > public review" and "[ ] needs core dev review", and a button with > "(Set me as a curator on this issue)", or maybe a text input for > curator email. "Set me as curator" already exists - it's called the ticket owner. "Needs core-dev review" is "ready for checkin". "Needs public review" is anything that has a patch, but isn't in "ready for checkin", and doesn't have the 'needs improvement' flag set. > That way, DDN stage might not be needed -- it's just "unreviewed" + > "needs core dev review". No -- DDN is a different issue. DDN means that there is a larger design issue that needs to be resolved. This might be resolved by core developer fiat, but more likely, it requires someone to start a discussion on django-dev. > Date of last ticket update can be also added to the trac reports -- to > sort tickets based on that date. Again, I'm not sure I see the importance of this. Ticket number gives you a sequential ordering (which lets you hit "new" issues"); beyond that, any bug is a bug, and needs to be fixed. > A robot can be written to fill these fields based on other fields, > comments authors, and knowledge of who are in core dev team. I would > also consider an external service for reviewing and triaging, but that > way you won't see review states in trac. To me, this points to a fairly glaring logical fallacy. If a robot can fill in new "reviewing" fields based on existing data, then doesn't that mean that the existing data is sufficient to determine reviewing state? Yours, Russ Magee %-) -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
