Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Fix issues#13 (add `patch_admin` function) #14

Merged
merged 5 commits into from Mar 22, 2013

Conversation

Projects
None yet
2 participants
Contributor

pahaz commented Mar 20, 2013

#13

Contributor

pahaz commented Mar 20, 2013

Need fix tests >_<. And restart travis.

TemplateSyntaxError: Could not parse the remainder: ':index' from 'admin:index'. The syntax of 'url' changed in Django 1.5, see the docs.

Owner

jedie commented Mar 21, 2013

Don't know if this the only change needed to support Django 1.5... So i set it to v1.4.x, see: fff7c98

force_text would not be used anywhere...

Owner

pahaz replied Mar 21, 2013

Yes, Sorry.

Two ideas here:

  1. Why not remove the chech and directly try to unregister and catch if this not work?
  2. We can also ignore if the model is not registered yet and just skip the unregister
Owner

pahaz replied Mar 21, 2013

It is best to inform the user about what he is doing something wrong. If you make the decision yourself, we can get unexpected side effects. Always better than the implicit explicit. If the user calls a function with invalid parameters - likely he made a mistake it is always better to warn of possible errors.

Owner

jedie commented Mar 21, 2013

I add some comments in the commit, see: pahaz/django-reversion-compare@bd011c7

Can you also insert in the README a short example, please?

jedie added a commit that referenced this pull request Mar 22, 2013

Merge pull request #14 from pahaz/patch-2
add `patch_admin` function
Thx pahaz!

@jedie jedie merged commit 9235a96 into jedie:master Mar 22, 2013

1 check passed

default The Travis build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment