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

Port to python3 #13

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Conversation

Abhay-dot
Copy link

No description provided.

@Abhay-dot
Copy link
Author

Ported this repo and tested it, the logs exit with 0 errors [GCI]

Screen Shot 2020-01-19 at 21 48 13

@quozl
Copy link
Contributor

quozl commented Jan 19, 2020

One of the print statements has a doubling of parentheses; so I'm not sure if you reviewed each change proposed by 2to3, or if you tested that statement.

@Abhay-dot
Copy link
Author

Abhay-dot commented Jan 19, 2020 via email

@quozl
Copy link
Contributor

quozl commented Jan 20, 2020

Tried to test. It doesn't work.

$ sugar-activity3
Traceback (most recent call last):
  File "/usr/bin/sugar-activity3", line 5, in <module>
    activityinstance.main()
  File "/usr/lib/python3.7/dist-packages/sugar3/activity/activityinstance.py", line 179, in main
    module = __import__(module_name)
  File "/usr/share/sugar/activities/Reflect.activity/activity.py", line 45, in <module>
    import telepathy
ModuleNotFoundError: No module named 'telepathy'
$ █

Please see our Python 3 Porting Guide for everything that must be checked.

@Abhay-dot
Copy link
Author

Abhay-dot commented Jan 20, 2020 via email

@quozl
Copy link
Contributor

quozl commented Jan 20, 2020

Previous work by others, is probably the best. You would look at activities that import TelepathyGLib and evaluate the commits at a diff detail level.

@Abhay-dot
Copy link
Author

Hey, I did a few changes using the python porting guide , but there are a few places where this is channel used, i'm not aware of the dbus api. It would be really nice of you if you could review the changes and tell me what else i can do.

@quozl
Copy link
Contributor

quozl commented Jan 22, 2020

Thanks.

You've changed the import in activity.py but the references to telepathy have not changed; so this won't work. It looks like you have not tested collaboration. What testing environment do you have setup for collaboration? You must test what you write.

You changed only CHANNEL, which was an example in the guide to porting. In addition to CHANNEL, every other constant that is imported from the static binding from telepathy import has to change to the corresponding constant from the from gi.repository import TelepathyGLib scope. The constants will be identical in value, but often different in name. (Knowing they will be identical is helpful, because you can run an interactive Python interpreter, import both bindings and experiment with dir().)

However, the source code in the file textchannelwrapper.py is maintained in the CollabWrapper repository, and has already been ported to Python 3 in that repository.

Also, in porting to the latest CollabWrapper, there is no need for any import or use of Telepathy API in the activity.py file.

But the CollabWrapper API may have changed since textchannelwrapper.py was last maintained. Hence collaboration must be tested.

Copy link
Member

@srevinsaju srevinsaju left a comment

Choose a reason for hiding this comment

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

Nice work :) Some minor changes

activity.py Show resolved Hide resolved
activity.py Outdated Show resolved Hide resolved
reflectwindow.py Outdated Show resolved Hide resolved
@quozl
Copy link
Contributor

quozl commented Jan 24, 2020

Thanks. Good progress. I see that the port to TelepathyGLib is not yet complete (static imports of telepathy are present), so I'll wait for that.

@srevinsaju
Copy link
Member

Hey, I did a few changes using the python porting guide , but there are a few places where this is channel used, i'm not aware of the dbus api. It would be really nice of you if you could review the changes and tell me what else i can do.

@Abhay-dot I have ported collabwrapper and some telepathy, ported Gconf to Gio.Settings, and some more fixes on Abhay-dot#1. Please review.
I tested the current progress, there are a lot of errors so far within the python3 port alone, but I'm pretty sure we will be able to fix them. You can continue working after merging the PR, so you can checkout the errors and try to fix them,

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