Skip to content
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

tools: Port to Python 3 #9500

Merged
merged 2 commits into from
Jun 26, 2018
Merged

Conversation

martinpitt
Copy link
Member

@martinpitt martinpitt commented Jun 25, 2018

Keep tap-{gtester,driver} as python 2, as they are also run on
RHEL/CentOS images. But they also work with Python 3 now.

  • image-refresh ubuntu-1604

@martinpitt martinpitt changed the title WIP: tools: Port to Python 3 tools: Port to Python 3 Jun 26, 2018
martinpitt added a commit to martinpitt/cockpit that referenced this pull request Jun 26, 2018
Keep tap-{gtester,driver} as python 2, as they are also run on
RHEL/CentOS images. But they also work with Python 3 now.

Closes cockpit-project#9500
@martinpitt martinpitt added bot and removed needswork labels Jun 26, 2018
@martinpitt
Copy link
Member Author

This needs a new ubuntu-1604 image to get python3 into its pbuilder. The other images already have it.

@cockpituous
Copy link
Contributor

image-refresh in progress on cockpit-tasks-zgqmz.
Log: http://fedorapeople.org/groups/cockpit/logs/image-refresh-9500-20180626-103232/

@cockpituous cockpituous changed the title tools: Port to Python 3 WIP: cockpit-tasks-zgqmz: tools: Port to Python 3 Jun 26, 2018
@cockpituous
Copy link
Contributor

@cockpituous cockpituous changed the title WIP: cockpit-tasks-zgqmz: tools: Port to Python 3 tools: Port to Python 3 Jun 26, 2018
@martinpitt
Copy link
Member Author

@Gundersanne : I assigned this to you for review, to get you into the mood and show you around more places of Cockpit. This is a rather easy one which self-tests. For any review, please don't shy away to ask about anything that's unclear - often these are a sign that some change needs a better commit message, or a comment, or needs more legible code, and so on.

Copy link
Contributor

@croissanne croissanne left a comment

Choose a reason for hiding this comment

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

Looks good, just one small question.

@@ -27,7 +27,7 @@ from glob import glob

proj_dir = os.path.dirname(os.path.dirname(os.path.realpath(sys.argv[0])))

max_version = 0
max_version = ''
Copy link
Contributor

Choose a reason for hiding this comment

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

A bit further on there's a max_version == 0 check. Won't that always be False now?

Copy link
Member Author

Choose a reason for hiding this comment

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

Well spotted, thanks! Fixed.

Keep tap-{gtester,driver} as python 2, as they are also run on
RHEL/CentOS images. But they also work with Python 3 now.

This requires an updated ubuntu-1604 image with python3 in the pbuilder
chroot (the other images already have it).

Closes cockpit-project#9500
Copy link
Contributor

@croissanne croissanne left a comment

Choose a reason for hiding this comment

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

Thanks 👍

@martinpitt martinpitt merged commit dd8e2fb into cockpit-project:master Jun 26, 2018
@martinpitt martinpitt deleted the tools-py3 branch June 26, 2018 21:18
gnomesysadmins pushed a commit to GNOME/gnome-keyring that referenced this pull request Aug 25, 2018
gnomesysadmins pushed a commit to GNOME/gnome-keyring that referenced this pull request Aug 27, 2018
This replaces tap-driver and tap-gtester with fresh versions from Cockpit.

cockpit-project/cockpit#9500
https://wiki.gnome.org/Initiatives/GnomeGoals/Python3Porting

Also returned the following commits that are not included in Cockpit:

* 918ab83
* 7e75a62
* 7120f44

Maybe they should be upstreamed.
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.

None yet

3 participants