Skip to content

Conversation

DasFranck
Copy link
Contributor

There's some few things which doesn't works with python3.
Quick-fixed.

try:
from StringIO import StringIO
except:
pass
Copy link
Member

Choose a reason for hiding this comment

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

i'm struggling to understand how pass can work here. isnt some kind of StringIO import required in order for the code to be able to work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My bad, I'm fixing this right now.

@dsjoerg
Copy link
Member

dsjoerg commented May 21, 2016

Hi, reading the code I have only one question which I asked in-line. In general: does the code pass unit testing with python2 and/or python3?

@DasFranck
Copy link
Contributor Author

DasFranck commented May 21, 2016

Both of ggtracker and my fork failed on test_replays with Python3 because of an invalid syntax (Python2 print syntax)

File "/tmp/sc2reader/test_replays/test_all.py", line 441
  print "DOING {}".format(i)

For test_s2gs, it works on my side while it doesn't on the ggtracker version, because of what I've changed.

Nothing seems to have changed for Python2.

@dsjoerg
Copy link
Member

dsjoerg commented Jun 9, 2016

Hi @DasFranck sorry for the delay.
When you say "Nothing seems to have changed for Python2." you mean that in Python2, the same tests that passed before your change still pass after your change?

@DasFranck
Copy link
Contributor Author

Yes, that's exactly what I meant.

@dsjoerg dsjoerg merged commit bdf3bc2 into ggtracker:upstream Jun 10, 2016
@dsjoerg
Copy link
Member

dsjoerg commented Jun 10, 2016

@DasFranck so funny we should talk about this, someone just asked on twitter about python3 support and now I can tell them we do! Thank you.

@choucavalier
Copy link

good job @DasFranck 👍

@DasFranck DasFranck deleted the upstream branch April 4, 2017 09:58
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