-
Notifications
You must be signed in to change notification settings - Fork 50
Adds support for Python 3 #6
base: master
Are you sure you want to change the base?
Conversation
del m['options'] | ||
m['name'] = m['name'].strip() | ||
original_team = original.get('team') | ||
team = original_team if original_team is not None else default_team | ||
severity = original.get('severity') or 'CRITICAL' | ||
if team: | ||
if isinstance(team, basestring): | ||
if isinstance(team, str): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this will break Python 2, since we get unicode
objects out of the yaml file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I completely overlooked that. I pushed another commit in the meanwhile, which should fix that.
Any reasons why the pull request is still open? |
Going forward, it seems unnecessary effort to me to support Python 2 and 3 at the same time, and since we don't have unit test coverage (yet) - why take the chance. What's your use case where you can use Python 3 but not Python 2? |
Yes sure support both is way to much effort. It's not a real argument but python3 is the future 🚀 |
When can we expect to get this merged? The complexity of the patch involving list() and print() seems perfectly reasonable. |
As DogPush didn't support Python 3 this pull request adds support for it.