Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make medium use a label as identifier #36

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sherzberg
Copy link

fixes #21 this commit uses the backends label from NOTIFICATION_BACKENDS setting
for easier to rearrange backends

for upgrades, you may have to write a custom alter statement to turn integers into varchars and then change the labels in your NOTIFICATION_BACKENDS to the string equivalent of the order

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling b832c3f on sherzberg:medium_id-is-label into 6782099 on pinax:master.

@sherzberg sherzberg closed this Jun 16, 2015
@paltman
Copy link
Member

paltman commented Jun 19, 2015

@sherzberg I actually like this change. Sorry for not merging it sooner.

@paltman paltman reopened this Jun 19, 2015
@paltman
Copy link
Member

paltman commented Jun 19, 2015

@sherzberg i'll try to update it and get it merged but if you could do me a favor and rebase your branch to get it updated so that I can merge it through GitHub it might make things faster. Thanks.

this commit uses the backends label from NOTIFICATION_BACKENDS setting
for easier to rearrange backends

Conflicts:
	notification/backends/__init__.py
	pinax/notifications/models.py
@sherzberg
Copy link
Author

@paltman cool! i rebased and had to fix a few conflicts. tests seem to pass locally on python 2.7 and django 1.7-1.8. I dont have python 3 setup on my home machine, so just waiting to see if the the travis build works out ok.

@ossanna16
Copy link

@paltman Can this be merged?

@psychok7
Copy link

@sherzberg i tested this PR with python3 and it works.. hope it gets merged sometime

psychok7 referenced this pull request in Ubiwhere/pinax-notifications Nov 29, 2016
@psychok7
Copy link

psychok7 commented Dec 7, 2016

@paltman @ossanna16 can this be merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NoticeSetting.medium should be IntegerField instead of CharField
5 participants