peopledoc / django-mail-factory

Django Mail Manager
Other
72 stars 17 forks source link

Do not load url templatetag in templates #76

Closed wo0dyn closed 7 years ago

wo0dyn commented 7 years ago

We had this warning:

RemovedInDjango19Warning: Loading the url tag from the future library is deprecated and will be removed in Django 1.9. Use the default url tag instead.

hsmett commented 7 years ago

Hi @wo0dyn ! As the url tag is part of the built-in templatetags (django/template/defaulttags.py), there should be nothing to import. Rendering this template would raise an error like 'url' is not a registered library. You can simply get rid of the load url.

wo0dyn commented 7 years ago

Oh s…! Thank you very much @hsmett. I will amend the PR.

hsmett commented 7 years ago

You may add some tests? :p

wo0dyn commented 7 years ago

You may add some tests? :p

Spotted. 👍 I thought it was too simple to write a test… My mistake. 😉

wo0dyn commented 7 years ago

Is it still OK for you guys? @hsmett @Natim

Natim commented 7 years ago

Yes it is good to me, also it is generally a bad idea to squash commits because it removes all the work of collaboration that we have been doing. Your work, then @hsmett reviews and then my reviews doesn't appear if you squash the commit. While it was a process that lasted 17 hours, we don't have any track of it anymore.

More about that has been written by @benoitbryon back in the days:

wo0dyn commented 7 years ago

it is generally a bad idea to squash commits because it removes all the work of collaboration that we have been doing

Oops, sorry about that. 😕

FYI, here is a small set of commits from my reflog prior to rebasing:

Natim commented 7 years ago

It isn't a big deal I think.

wo0dyn commented 7 years ago

More about that has been written by @benoitbryon back in the days

Thanks for the blog posts. Will read that later.