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

Removed __unicode__() in mongonaut_tags.py. Added a unit test #80

Merged
merged 2 commits into from
Dec 7, 2015
Merged

Conversation

lchsk
Copy link
Contributor

@lchsk lchsk commented Aug 13, 2015

Solution for the problem mentioned in #79 with a unit test

@@ -32,7 +32,7 @@ class NewUser(User):
new_field = StringField()

def __unicode__(self):
return self.email
return unicode(self.first_name)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this changed to first_name? It no longer matches the User class above. I'd prefer this continue to be the email.

Also, why is this getting caste to unicode?

@lchsk
Copy link
Contributor Author

lchsk commented Aug 14, 2015

I've used first_name so it would make more sense to put non-ascii characters there. But I can change it back to email. I'll send a pull request afterwards.

@garrypolley
Copy link
Collaborator

@lchsk No big deal. I just noticed that was the example application. No need to change it.

@garrypolley
Copy link
Collaborator

I'm cool with these changes. 👍 I'll give some others a day or so to look these over and then I'll merge it in.

Thanks for the pull request.

@pydanny
Copy link
Member

pydanny commented Aug 14, 2015

I think stronger support of unicode is always a good thing. I agree with @garrypolley that we shouldn't switch from email.

@garrypolley
Copy link
Collaborator

@lchsk Are you currently using what is in this pull request? It's not had much activity for months.

If you need it I can hit the merge button, but I'd like to see the email used as mentioned above.

@lchsk
Copy link
Contributor Author

lchsk commented Dec 5, 2015

Hi
I totally forgot about this (I'm embarrassed enough :(). But yes, I use version from the pull request. I removed unnecessary changes I had made previously. I would be great if you could merge it. Cheers

@garrypolley
Copy link
Collaborator

Never feel embarrassed, thanks for the contribution! We appreciate any help we can get on open source projects.

garrypolley added a commit that referenced this pull request Dec 7, 2015
Removed __unicode__() in mongonaut_tags.py. Added a unit test
@garrypolley garrypolley merged commit f046a55 into jazzband:master Dec 7, 2015
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.

3 participants