Hi, J. Alheid, Don't be too afraid to get started. From looking at the history on that ticket (as well as the current codebase), my guess is that Jacob's concern is that there's no mechanism in place for handling what happens to those objects if commit=False. The current patch tells us what *not* to do with those objects, but doesn't do something else with them instead. Obviously it's not good to go ahead and delete anyway if commit is not True, but just bypassing that obj.delete() call would raise a completely different problem.
The best way to see if that concern is valid is to write the necessary regression test for the patch. It should verify that under the current trunk the objects are deleted when they should *not* be, and that with the patch applied, the objects are not only not deleted on the first pass when commit=False, but that they are subsequently deleted when they should be later on. Offhand, I think I agree with Jacob that the existing patch will cause problems on the latter part of that test case. I'm sure he knows better than I, though. Anyhow, that's what it needs. Oh, and btw, the chunk of code has moved in the current trunk. The delete call is now at: http://code.djangoproject.com/browser/django/trunk/django/forms/models.py#L604 So the patch itself needs updating regardless. All the best, - Gabriel On Jun 2, 12:49 pm, Shakefu <[email protected]> wrote: > Hi, I'd really like to get this ticket resolved, and I'm willing to do > the work personally, but I've never contributed before, so I was > hoping to solicit some guidance. > > It seems to me that we'd need a similar patch to the one already > existing but updated against trunk and a patch for the documentation > to note the change in behavior. I commented last night (Wedg) and > someone set the needs tests flag, but I'm not entirely sure what tests > would be needed, except maybe to validate that commit=False doesn't > delete instances, and commit=True does. > > I dunno if I'm a bit too green with the contributing to take ownership > of this ticket, but I'd like to do anything I can to help get this > ticket's suggested behavior into trunk, for my own selfish reasons. :D > > Please advise, > J. Alheid -- You received this message because you are subscribed to the Google Groups "Django developers" 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/django-developers?hl=en.
