-
Notifications
You must be signed in to change notification settings - Fork 237
Transition to Python 3.x #191
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
Conversation
buildPy2exe.py
Outdated
| CreateShortCut "$$SMPROGRAMS\Syncplay\Syncplay Server.lnk" "$$INSTDIR\syncplayServer.exe" "" | ||
| CreateShortCut "$$SMPROGRAMS\Syncplay\Uninstall.lnk" "$$INSTDIR\Uninstall.exe" "" | ||
| WriteINIStr "$$SMPROGRAMS\Syncplay\SyncplayWebsite.url" "InternetShortcut" "URL" "https://syncplay.pl" | ||
| WriteINIStr "$$SMPROGRAMS\Syncplay\SyncplayWebsite.url" "InternetShortcut" "URL" "http://syncplay.pl" |
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.
Is there a reason why this was changed to http?
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.
EDIT: fixed. I was looking at the wrong URL, my bad. Thanks for spotting this.
| def prepareInstallListTemplate(self, fileList): | ||
| create = [] | ||
| for dir_ in fileList.iterkeys(): | ||
| for dir_ in fileList.keys(): |
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.
In python3 dictionary iteration, it's often better to use for key in dict rather than for key in dict.keys(). dict.keys() returns a list rather than a generator-like object, which for key in dict does.
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 retract my statement. It's essentially the same. I think dict in keys is still faster in most cases.
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.
Lot of these changes were done by 2to3. If you want, I can edit this one, but I guess the same issue will appear in multiple other parts of the codebase. If you think these could cause potential problems, I can scroll all the changes and apply the same fix everywhere.
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 see. Yeah there's a lot of places that could be improved with shorter notation, but they're not necessarily "wrong".
I think I'll just ignore them for now, and there can be another PR that cleans up the code.
syncplay/vendor/Qt.py
Outdated
| """ | ||
|
|
||
| assert isinstance(ptr, long), "Argument 'ptr' must be of type <long>" | ||
| assert isinstance(ptr, int), "Argument 'ptr' must be of type <long>" |
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.
Comment should be int and not long right?
|
@xNinjaKittyx Is everything okay now? |
|
@Et0h yeah |
Following the discussion of issue #169, this PR ports all the codebase to Python 3.x on all the platforms.
A few info and caveats:
Thanks to @Et0h and @xNinjaKittyx for their assistance and support during the port.