Hi Justin, Correct - this should be handled as two separate tickets.
If the patch for one ticket depends on a second, it's fine to have that dependency in the patches -- just note in the comments for the ticket when you upload your patch, or in the comment for the pull request. Essentially, we just need to make sure that whoever reviews the second patch knows that the other patch needs to be reviewed and integrated first. Plus, if you put in a link the other way as well, the person who reviews the first patch will know that there's a second patch in a closely related area that would be worth reviewing while they're in the same mental space. Yours, Russ Magee %-) On Sun, Jul 28, 2013 at 11:54 AM, Justin Michalicek <[email protected]>wrote: > Awesome, thank you. I am guessing it is preferred that these be done as > two separate tickets and patches since they are technically separate > features. I'm not seeing an obvious way to mark one as relying on another, > though. Should I just note in the trac ticket for #2 that it relies on #1 > when I create the tickets, wait for #1 to be accepted and merged then do > the PasswordResetForm, or just do them both in one feature branch and pull > request due to the PasswordResetForm change relying on the change to > send_mail()? > > > On Saturday, July 27, 2013 11:17:48 AM UTC-4, Justin Michalicek wrote: >> >> I would like to make my first contribution to the Django code, but these >> both are pretty easy and at least to me seem pretty obvious, so I thought >> there might be some good reason they have not yet already been done. >> Depending on the response, I would do these in different ways as well, so I >> want to check here before getting to work. >> >> 1) Adding an html_message parameter to django.core.mail.send_mail(), the >> same as mail_admins() and mail_managers() have. Seems useful and like a >> good idea, but it's dead simple, so it also seems like it would already be >> there unless it for some reason had been decided that this is absolutely >> not going to happen. >> >> 2) Adding the ability for django.contrib.auth.views.**password_reset() >> to take an optional parameter for an html email template as well as >> django.contrib.auth.forms. >> >> PasswordResetForm.save() so that html password reset emails may be sent. >> Again, simple enough that I worry that there's a reason this isn't already >> done that I'm unaware of. >> It is also somewhat dependent on #1 there in that either #1 has to be done >> first or this has to be done by using EmailMultiAlternatives directly rather >> than send_mail(). >> >> Any input on if I should proceed or if there's some reason I should not >> touch these would be greatly appreciated. I'd love to make a contribution >> to Django and these look >> like a safe, simple, and useful place to start. >> >> -- > You received this message because you are subscribed to the Google Groups > "Django developers" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > To post to this group, send email to [email protected]. > Visit this group at http://groups.google.com/group/django-developers. > For more options, visit https://groups.google.com/groups/opt_out. > > > -- You received this message because you are subscribed to the Google Groups "Django developers" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at http://groups.google.com/group/django-developers. For more options, visit https://groups.google.com/groups/opt_out.
