Skip to content

make medium use a label as identifier - #36

Open
sherzberg wants to merge 1 commit into
pinax:masterfrom
sherzberg:medium_id-is-label
Open

sherzberg wants to merge 1 commit into
pinax:masterfrom
sherzberg:medium_id-is-label

Conversation

@sherzberg

Copy link
Copy Markdown

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
Copy Markdown

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

paltman commented Jun 19, 2015

Copy link
Copy Markdown

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

@paltman paltman reopened this Jun 19, 2015
@paltman

paltman commented Jun 19, 2015

Copy link
Copy Markdown

@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
sherzberg force-pushed the medium_id-is-label branch from b832c3f to 057a311 Compare June 19, 2015 04:33
@sherzberg

Copy link
Copy Markdown
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
Copy Markdown

@paltman Can this be merged?

@psychok7

Copy link
Copy Markdown

@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

psychok7 commented Dec 7, 2016

Copy link
Copy Markdown

@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