Conversation
- make sure there is really a zerg in that replay, otherwise the test would test nothing at all - remove outdated comment, that test does not fail.
|
Ah so it seems not only did So I am also trying to fix that now... |
|
I think the s2gs functionality of this library may be safely removed. We added s2gs parsing back in 2012 because back then s2gs was a valuable source of information about matches for GGTracker. But the only way to get s2gs files en masse is to automate the SC2 client, and I'm not aware of anyone doing that kind of thing these days. |
|
Thanks @dsjoerg for this comment. I propose to have a dedicated issue, in order to discuss the removal of an unused feature. Since the purpose of this PR was to make some existing tests run again. |
| # Newer unittest features aren't built in for python 2.6 | ||
| import sys | ||
|
|
||
| if sys.version_info[:2] < (2, 7): |
There was a problem hiding this comment.
Maybe not for this PR but do we really need to support Python versions < 2.7 -- They have been EOL for 5+ years.
https://devguide.python.org/devcycle/#end-of-life-branches
There was a problem hiding this comment.
I personally totally agree here.
There was a problem hiding this comment.
https://github.com/ggtracker/sc2reader#installation says 2.6 is supported! My vote would be to drop all legacy Python support.
There was a problem hiding this comment.
Agreed. Should I open a dedicated issue for this? So others can have a word?
|
Should we merge this? |
|
Are the format modification made by python/black? |
|
yes. Sorry .. was maybe a bit forward of me. Should I remove that? |
|
I am a fan of python/black but others are free to disagree. |
|
Looks good to me. Thanks for fixing this! I am also okay using black. I'll make an issue for it |
4 participants
should fix #78
For reviewers:
black -l 127