On Tue, Oct 5, 2010 at 9:58 AM, Tai Lee <real.hu...@mrmachine.net> wrote: > Hi Russ, > > On Oct 5, 11:48 am, Russell Keith-Magee <russ...@keith-magee.com> > wrote: > >> > Perhaps we need another triage stage for these tickets, "Needs final >> > review" or something? >> >> That's essentially what RFC is. This is an area where our documented >> procedures have lagged behind in-practice procedures, but we've >> essentially accepted that once a patch has been written by one person, >> and reviewed by another and found to be ok, then it's RFC, which means >> a core committer needs to take a look. > > I'm happy for RFC to indicate that a ticket is ready for final review > by a core committer, after being reviewed by at least someone other > than the patch author. But, I think there's still a problem of > accepted tickets that have a patch with docs and tests and no feedback > indicating that it needs improvement, being lost in the shuffle, as > well as how and why we ever end up with tickets in this state and how > can the patch author get someone to review their work.
You can get a list of tickets that meet that criterion very easily - it's tickets in "accepted" with a "patch needs improvement" flag. This doesn't solve the problem of people who review a ticket, but don't flick the "patch needs improvement" flag or provide feedback on what exactly is wrong... but no technological solution will fix those problem. The best we can hope for here is for a second reviewer to come along and clean up the ticket state. > It could happen that a ticket is accepted on merit, then it gets a > patch with docs and tests, and then sits there getting no attention > because nobody knows it needs a review. The patch author could just > move it to RFC themselves as it had previously been accepted and he or > she subsequently added the required docs and tests, in order to get > somebody to look at it. But I think this is generally frowned upon and > might take up too much core committer attention before the ticket is > really ready. > > It could also happen that a ticket is unreviewed and already has a > patch with docs and tests, if it is reviewed by a core committer (or > not) and "accepted" without feedback about any necessary improvements, > shouldn't it have simply gone straight to RFC instead? What should the > original patch author do when his or her patch with docs and tests is > reviewed by someone else and moved to "accepted" like this? There's no > feedback that they need to incorporate into the patch, and they can't > move their own work to RFC, even though someone else has reviewed and > "accepted" the patch? > > I'd like to see the ticket system work smoothly in and of itself, and > for tickets to be reviewed without the patch authors having to ask > around on IRC or django-developers for someone to take a look. I don't > want to make the ticket system too complicated, but I think it is > already a bit muddled by determining whether or not a ticket needs > review by checking 5 or 6 different properties, instead of a single > clear indicator set by the patch author that he or she would like > somebody (not necessarily a core committer) to review the work. This > could also help in eliminating another problem of not knowing if a > ticket is still being worked on or if the patch author considers it > finished. The problems you describe here fall into a couple of categories: * People doing triage, but not setting flags or giving feedback correctly. This is a social problem; no technology is going to fix it -- especially not adding *more* flags to our Trac configuration. Improved documentation of what we expect *might* improve things here; suggestions (and patches!) are welcome. * Improving visibility of work that needs to be done. This is somewhere that Trac gardening is needed -- the relevant queries aren't hard to write; they just need to be made more prominent. * Determining which ticket out of hundreds require attention. Two ways to slice this that aren't currently available to us are ordering by last modification date, and ordering by "popularity" (as determined by users voting on tickets). Right now, the biggest impediment is that our Trac install is *really* old, so all the nice new plugins implementing these features won't run. We're in the process of upgrading Trac; once that is done, we'll be able to add some of these new features. The only edge case I can see that you've identified that we don't cover is people who upload incomplete patches which they *don't* want reviewed yet. I don't see this as a major problem -- in my experience, people upload patches because they think they're ready. Patches may not always be correct, but I haven't seen a lot of people using Trac as a temporary storage area for incomplete work. If someone uploads a patch, they're generally looking for *some* feedback, even if it's just "am I on the right track?" >> As a general principle, DDN isn't usually a situation where more >> information is needed; it's usually just time that is lacking. >> >> Sprints definitely help push this sort of thing along. However, that's >> more because we have two core developers in a room at the same time, >> not because of the presence of the rest of the community (although the >> community got lots of other great work done). Malcolm and I spend a >> day at the recent sprints just doing DDN tickets, and we managed to >> clear out quite a few DDNs. We made vague plans to hook up on >> IRC/Skype as a semi-regular activity to try and clear out the backlog, >> but we haven't made that happen yet. > > Are at least two core committers required for all DDN tickets? Can a a > single core committer make decisions on minor or trivial tickets, and > the reporter be given the option to request a vote or further > consideration if they feel the decision is wrong (possibly based on > lack of information)? Perhaps some of the people who are well known > and long standing in the community (but who don't have commit access) > could make some of these decisions? Are two core developers *required*? No. Having two developers is really just a way of maintaining momentum; both in the since that pair programming helps you maintain focus, and as a way to ensure that you get a quick sanity check on an idea, rather than bouncing it into the too-hard basket. Could trusted non-core people do some of this DDN work? Yes, and they already do. If you check the activity logs from yesterday, Eric Holscher moved some of DDN tickets relating to the testing system, as well as flicking a couple of tickets *to* DDN where he wasn't comfortable making a call. It's a little like jaywalking laws. Almost all cities have jaywalking laws on the books. If you're visiting a city, you'll generally err on the side of caution and not jaywalk at all. The longer you stay, the better you will understand when the police *really* care about jaywalking, when local traffic will be on the lookout etc. You won't find "official" rules for when it's ok to jaywalk, but people do it anyway, nobody is harmed, everyone gets where they need to go in a timely fashion, and life goes on. The same goes with Trac. We have an official set of Trac guidelines. However, our Trac install is deliberately open ended so that anyone can do anything to any ticket. We trust (and so far, that trust has been generally well placed) that people won't start making decisions arbitrarily. Trusted members of the community generally know when and where they are trusted (like Eric, especially when it comes to testing), and we rely on those people being a little bold and jaywalking occasionally. 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.
