search

translate / virtaal

Easy-to-use and powerful offline translation tool
https://virtaal.translatehouse.org
GNU General Public License v2.0
298 stars 57 forks source link

Port to Python 3 #3306

Closed friedelwolff closed 4 years ago

friedelwolff commented 4 years ago

I'm creating this pull request to merge @licamla's work into an upstream branch.

This serves to show official activity in the upstream repository so that anybody interested can know this is real project progress. Also, I have a number of additional improvements to commit, but should go on top of this work. This isn't ready for master yet, but can hopefully be ready soon.

From now on, let's continue the work there, and create pull requests for the py3 branch. Of course, this his also for the port to GObject introspection and GTK3, not only for Python 3.

Many, many thanks for the work!

friedelwolff commented 4 years ago

I don't think a review is actually appropriate here, but I thought I should just comment on a few things to make the future commits fit in well. I'll merge this anyway! I'm making a few review comments on a few things that you can hopefully answer when you have some time.

Thank you again for all the work. It was just the motivation I needed to get back to this as well.

llimeht commented 4 years ago

This is great to see. Thanks for this fantastic contribution @licamla!

In the interests of seeing what the Debian packaging would need to look like for an updated Virtaal, I've taken a look at packaging the py3 branch from @licamla. The first impression is "looks good" and I might even upload it to the experimental repo as it is.

Of course, someone else hammering at a package with different tools and looking to complete different tasks (like packaging) is a great way of finding some bugs and I have some already. There's not really an easy way of reporting those bugs or offering bugfixes until this PR is merged, so perhaps that is the next step.

llimeht commented 4 years ago

(Incidentally for virtaal/plugins/_python_console.py there might be merit in going back to the source of that plugin at https://github.com/GNOME/rhythmbox/blob/master/plugins/pythonconsole/pythonconsole.py and seeing what their port looks like; there has been significant divergence over time on both sides so I don't know how feasible it is to just pick up the new code.)

licamla commented 4 years ago

@friedelwolff For the next few days, I was in another city. So I had no access to my development computer and no time to edit the code. I can only spare some time to reply to the review.

licamla commented 4 years ago

I don't think a review is actually appropriate here, but I thought I should just comment on a few things to make the future commits fit in well. I'll merge this anyway! I'm making a few review comments on a few things that you can hopefully answer when you have some time.

Just check before committing that commits are clean. The many future imports in setup.py could have been avoided :-) It looks like you are editing the Glade .ui file manually. I'm not quite that brave, but I really like it - otherwise it can become a bit hard to review those changes. This said: please pay careful attention to indentation, so that the indentation still reflects the structure accurately. Although I want to keep the changes small for now until we have feature parity, feel free to remove entirely useless code (like workarounds for old GTK2 bugs). I'm going to announce the porting effort on the translate-devel mailing list and thank you there too. I don't know if you are on that mailing list, but maybe some more discussions about future direction could take place there.

Thank you again for all the work. It was just the motivation I needed to get back to this as well.