attempting to merge my 1.7-friendly updates

arneb / django-generic-confirmation

A generic change confirmation app for Django

BSD 3-Clause "New" or "Revised" License

20 stars 12 forks source link

attempting to merge my 1.7-friendly updates #3

Closed lathropd closed 9 years ago

lathropd commented 9 years ago

Hi arne,

Made changes here to allow it to run under django 1.7. A bunch of it just ends up being whitespace cruft (sorry).

Actual changes:

using django.utils.timezone instead of datetime from the standard library
changed import from django.conf.urls in urls.py to be 1.7-compliant

All the tests now pass and I have a basic test app working.

I think we're going to be using this in production under 1.7 at The Dallas Morning News labs team, so if you'd like any help maintaining it, I'd love to pitch it with patches, documentation, whatever.

-D

arneb commented 9 years ago

Hi,

the changes are looking good to me. I have only one question: Is there any reason for dropping the "commit" argument from the resume_form_save method? https://github.com/lathropd/django-generic-confirmation/commit/8a5966926d89fdeb1f58dd506a40bcb0c2d633d5#diff-d76e2b385645985218e19046de9cef94L46

I will test the changes myself before merging (maybe tomorrow), but I will definately merge the changes.

Thank you for the contribution!

lathropd commented 9 years ago

Thank-you for writing it!

On the "commit" parameter, I think that's just because I was originally working from an older version. If you want, I can fix that and wrap everything into a new pull request.

Sent from my iPhone

On Nov 19, 2014, at 1:07 PM, Arne Brodowski notifications@github.com wrote:

Hi,

the changes are looking good to me. I have only one question: Is there any reason for dropping the "commit" argument from the resume_form_save method? lathropd@8a59669#diff-d76e2b385645985218e19046de9cef94L46

I will test the changes myself before merging (maybe tomorrow), but I will definately merge the changes.

Thank you for the contribution!

— Reply to this email directly or view it on GitHub.

arneb commented 9 years ago

I've fixed the commit parameter and merged your pull request. Thank you! Are you planning on contributing more changes in the near future? I'm thinking about making a new release for pypi (due to the django 1.7 compatiblity)...

lathropd commented 9 years ago

I would like to make some additions to the documentation to deal with the pitfalls I hit. When I finish my current project there might be some things worth pulling out and contributing as well.

What's your release timeline?

Daniel Lathrop 206.718.0349 (cell)

On Thu, Nov 20, 2014 at 2:42 AM, Arne Brodowski notifications@github.com wrote:

I've fixed the commit parameter and merged your pull request. Thank you! Are you planning on contributing more changes in the near future? I'm thinking about making a new release for pypi (due to the django 1.7 compatiblity)...

— Reply to this email directly or view it on GitHub https://github.com/arneb/django-generic-confirmation/pull/3#issuecomment-63776597 .

arneb commented 9 years ago

I have no fixed release-schedule and can postpone the release to january or so. Feel free to take your time and I make the release once your changes are merged.

lathropd commented 9 years ago

Sounds good!

-D

Daniel Lathrop 206.718.0349 (cell)

On Fri, Nov 21, 2014 at 10:40 AM, Arne Brodowski notifications@github.com wrote:

I have no fixed release-schedule and can postpone the release to january or so. Feel free to take your time and I make the release once your changes are merged.

— Reply to this email directly or view it on GitHub https://github.com/arneb/django-generic-confirmation/pull/3#issuecomment-63998072 .