On Fri, Jan 23, 2009 at 12:38 AM, Julien Phalip <jpha...@gmail.com> wrote:
> > Hi, > > I just wanted to draw your attention to what appears to be a bug in > Django: the 'tell()' proxy is missing from the Windows-specific > implementation of TemporaryFile. This causes Django to crash when > manipulating the uploaded file with PIL, for example. Ticket #9344 > contains a patch to fix that. > I probably looked at that ticket initially, at least briefly. Here's a peek into what likely went on in my head: - I should look at that...though I don't know the code involved...nor much about PIL...still, it's Windows-specific, I've got Windows boxes to test on, many (most...all?) other committers don't. - Hmm....crash when manipulating uploaded file with PIL...do I know offhand how to recreate that...no....do I want to learn enough about PIL to dream up how to recreate it...not really. - Maybe the patch has a test? Oh, there are 2 patches...I wonder why? They're identical...wha? Oh, the first had a spacing error. But regardless, no test. - Should there be a test? Is this something that can't be tested? Is that why no test was provided? Is it blindingly obvious to anyone who knows the first thing about PIL how to recreate this problem and that's why no specifics on how to recreate were included? Dunno...this is too hard, let's find something easier to look at. At this point I really should have noted in the ticket what stopped me from doing anything with it, but I didn't. I'm bad that way, particularly when I get to a point of thinking that maybe it's my own lack of knowledge that's the problem. So, what that ticket was lacking for me to look at it more closely was specific instructions as to how to recreate the problem so I could verify the fix. Even better, a test integrated into the test suite, then it's clear to anyone looking later on what exact problem was fixed, and there is built-in protection against it breaking again. If it's not feasible for some reason to add a test to the test suite then a note indicating why no test is possible would help. Now, the fix may be trivial (and I'll agree it looks trivial), but I'm not going to check in anything without testing it. Been there, done that, broke things, try real hard not to do it any more. So I want to be able to see the problem myself before the fix and verify the problem is gone after the fix. > Now, I know that this is sort of an edge case, and I also know that > there are more important and more urgent matters at this moment. But > I'd be keen to hear what is the official (or tacit) policy for that > kind of small bug reports. There probably are a few other tickets in > that situation (#9404 is another example). So, what is the best way to > go for people interested in having them checked in? Is it simply by > bringing them up on this mailing list from time to time? If so, then I > can try again after 1.1 lands. > Best way to make sure "small" tickets do not get hung up on the way to checkin is to make them dead easy, even for someone who may not be intimately familiar with that area of the code, to understand the problem and verify the fix. Include tests integrated into the Django test suite that fail before the fix and pass afterward. If integrated tests aren't possible, explain why the fix should be checked in even without tests, and include manual recreation instructions so the person who is considering the fix knows how to test it manually. Karen --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@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 -~----------~----~----~----~------~----~------~--~---
