8000 fix: use print() function in both Python 2 and Python 3 by cclauss · Pull Request #18395 · electron/electron · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix: use print() function in both Python 2 and Python 3 #18395

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Jun 15, 2019
Merged

fix: use print() function in both Python 2 and Python 3 #18395

merged 5 commits into from
Jun 15, 2019

Conversation

cclauss
Copy link
Contributor
@cclauss cclauss commented May 21, 2019 •
edited
Loading

Description of Change

notes: Fix issues that will halt the runtime on modern Python.

Legacy print statements are syntax errors in Python 3 but print() function works as expected in both Python 2 and Python 3.

Old style exceptions are syntax errors in Python 3 but new style exceptions work as expected in both Python 2 and Python 3.

  • basestring was removed in Python 3 in favor of str.
  • file() was removed in Python 3 in favor of open() which works in both Python 2 and Python 3.

225 days until Python 2 end of life.

Checklist

Release Notes

Notes:

@electron-cation electron-cation bot added the new-pr 🌱 PR opened recently label May 21, 2019
@welcome
Copy link
welcome bot commented May 21, 2019

💖 Thanks for opening this pull request! 💖

We use semantic commit messages to streamline the release process. Before your pull request can be merged, you should update your pull request title to start with a semantic prefix.

Examples of commit messages with semantic prefixes:

  • fix: don't overwrite prevent_default if default wasn't prevented
  • feat: add app.isPackaged() method
  • docs: app.isDefaultProtocolClient is now available on Linux

Things that will help get your PR across the finish line:

  • Follow the JavaScript, C++, and Python coding style.
  • Run npm run lint locally to catch formatting errors earlier.
  • Document any user-facing changes you've made following the documentation styleguide.
  • Include tests when adding/changing behavior.
  • Include screenshots and animated GIFs whenever possible.

We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can.

@cclauss cclauss force-pushed the Python-print-function branch from 8d7fe1a to 80a3288 Compare May 21, 2019 22:46
@nornagon
Copy link
Contributor

I'm all for switching to Py3 but we very heavily depend on Py2 currently because Chromium's build scripts are all Py2. Are you aware of any efforts in Chromium to upgrade to 3?

nornagon
nornagon reviewed May 21, 2019
View reviewed changes
nornagon
10000
nornagon reviewed May 21, 2019
View reviewed changes
tools/linux/rewrite_dirs.py
@@ -4,6 +4,7 @@
# found in the LICENSE file.
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we use this any more either.

Copy link
Member

Choose a reason for hiding this comment

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

this gets used by pkg-config-wrapper, which i believe we still use to wrap around pkg-config to generate the correct include and library paths when cross-compiling using a sysroot

Copy link
Contributor

Choose a reason for hiding this comment

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

I couldn't find any reference to this script, can you link me to where it's used?

Copy link
Member

Choose a reason for hiding this comment

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

cc @deepak1556 i think you may know more?

Copy link
Member
@deepak1556 deepak1556 May 23, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

This is a pre GN thing, not needed anymore. https://github.com/electron/electron/blob/3-0-x/toolchain.gypi#L80

@cclauss cclauss force-pushed the Python-print-function branch from 80a3288 to dcb6beb Compare May 21, 2019 22:55
@cclauss
Copy link
Contributor Author
cclauss commented May 21, 2019

@nornagon None of these changes break Python 2 compatibility. There has been a lot of movement towards Python 3 compatibility in the Chromium codebase in recent months. Please check it if you get a chance.

nornagon
nornagon approved these changes May 21, 2019
View reviewed changes
Copy link
Contributor
@nornagon nornagon left a comment

Choose a reason for hiding this comment

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

Sweet. This looks good to me then!

@cclauss
Use print() function in both Python 2 and Python 3
454644d 
notes: Removed a series of Python 3 syntax errors
@cclauss cclauss force-pushed the Python-print-function branch from dcb6beb to 454644d Compare May 21, 2019 23:06
ckerr
ckerr approved these changes May 21, 2019
View reviewed changes
Copy link
Member
@ckerr ckerr left a comment

Choose a reason for hiding this comment

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

Thank you for writing this, @cclauss!

@cclauss
Copy link
Contributor Author
cclauss commented May 22, 2019

Am I correct in assuming that the build-mac failure is unrelated?

@MarshallOfSound
Copy link
Member

@cclauss Yup, that failure is due to the PR coming in from a fork. That test always fails for forks at the moment --> #17668

@electron-cation electron-cation bot removed the new-pr 🌱 PR opened recently label May 22, 2019
@codebytere
Copy link
Member

@cclauss can you resolve the merge conflicts? once that's done we can get this merged!

@codebytere codebytere merged commit 1d6e5e6 into electron:master Jun 15, 2019
@release-clerk
Copy link
release-clerk bot commented Jun 15, 2019

Release Notes Persisted

Fix issues that will halt the runtime on modern Python.

@welcome
Copy link
welcome bot commented Jun 15, 2019

Congrats on merging your first pull request! 🎉🎉🎉

@cclauss cclauss deleted the Python-print-function branch June 15, 2019 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nornagon nornagon nornagon approved these changes

@deepak1556 deepak1556 deepak1556 left review comments

@codebytere codebytere codebytere approved these changes

@ckerr ckerr ckerr approved these changes

@zcbenz zcbenz zcbenz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@cclauss 3868 @nornagon @MarshallOfSound @codebytere @ckerr @zcbenz @deepak1556
0