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 TelepathyGLib, collabwrapper, Gio.Settings #14

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

Port to TelepathyGLib, collabwrapper, Gio.Settings #14

wants to merge 7 commits into from

Conversation

ayushnawal
Copy link

Changes made in this PR:

  • Port from GConf to Gio.Settings
  • Port to TelepathyGLib
  • port statvfs to os module
  • update collabwrapper
  • update repository metadata

Tested the changes before and after changes. working as expected.

utils.py Outdated Show resolved Hide resolved
utils.py Outdated Show resolved Hide resolved
get_int for volume instead of get_string
removed path because it was used by GConf client
@ayushnawal
Copy link
Author

Done as Suggested.

@chimosky @quozl please review!

@chimosky
Copy link
Member

chimosky commented Mar 12, 2020

Tested, activity fails to start and logs show this error.

Traceback (most recent call last):
  File "/usr/bin/sugar-activity", line 5, in <module>
    activityinstance.main()
  File "/usr/lib/python2.7/dist-packages/sugar3/activity/activityinstance.py", line 179, in main
    module = __import__(module_name)
  File "/home/ibiam/Activities/reflect/activity.py", line 57, in <module>
    from reflectwindow import ReflectWindow
  File "/home/ibiam/Activities/reflect/reflectwindow.py", line 34, in <module>
    import utils
  File "/home/ibiam/Activities/reflect/utils.py", line 41, in <module>
    from jarabe import config
ImportError: No module named jarabe

jarabe is part of sugar and not the toolkit so it can't be imported, @quozl can better tell you how to fix it.

This is a good opportunity to port activity to Python 3, there's an existing work already in #13,maybe you can take a look at it.

Also edit your last commit message to better explain what you did, the header shouldn't be "changes made as requested".

Please try to test your changes.

@quozl
Copy link
Contributor

quozl commented Mar 12, 2020

My guess is that @ayushnawal was testing this alongside a Python 2 build of Sugar. That's fine, given that the activity has not yet been ported to Python 3. But we really need that port done, so #13 should be merged with this branch, and any further fixes applied.

I don't think this is pull request is ready for review and testing.

I also agree that commit messages could be clearer. See Making Commits.

@ayushnawal
Copy link
Author

My guess is that @ayushnawal was testing this alongside a Python 2 build of Sugar. That's fine, given that the activity has not yet been ported to Python 3. But we really need that port done, so #13 should be merged with this branch, and any further fixes applied.

@quozl I opened this PR with the purpose of porting it to Python 3 only 😁 and for the same, I have ported all the required dependencies to their respective latest form.

I asked for a review because the activity can be easily ported to python3 after these fixes are applied so I was waiting for this patch to be approved after testing and reviewing.

I don't think this is pull request is ready for review and testing.

Can changes up to this point be reviewed? so that I can port it quickly afterward.

I also agree that commit messages could be clearer.

Agreed. I will try to make them clearer now.

@chimosky
Copy link
Member

You can look at fixing the traceback above as activity fails to start.

@quozl
Copy link
Contributor

quozl commented Mar 16, 2020

@quozl I opened this PR with the purpose of porting it to Python 3 only and for the same, I have ported all the required dependencies to their respective latest form.
I asked for a review because the activity can be easily ported to python3 after these fixes are applied so I was waiting for this patch to be approved after testing and reviewing.

Okay, if that's your only purpose, we can wait until you have ported to Python 3, and then wait some more until the activity works properly.

Porting to Python 3 is not an end in itself. An activity must be usable, and mustn't have anything wrong with it, otherwise we wouldn't be able to release it and have people use it.

Can changes up to this point be reviewed? so that I can port it quickly afterward.

How would reviewing this unfinished work help our users? Some review has happened already above. See Guide for Reviewers for some other things that reviewers can do. Nothing springs to mind. What specifically are you wanting?

@ayushnawal
Copy link
Author

ported the activity to python3.

getting this in my log, as mentioned by @chimosky above

need help @quozl

Traceback (most recent call last):
  File "/usr/local/bin/sugar-activity3", line 5, in <module>
    activityinstance.main()
  File "/usr/local/lib/python3.6/dist-packages/sugar3/activity/activityinstance.py", line 179, in main
    module = __import__(module_name)
  File "/home/ayush/activities/reflect/activity.py", line 57, in <module>
    from reflectwindow import ReflectWindow
  File "/home/ayush/activities/reflect/reflectwindow.py", line 34, in <module>
    import utils
  File "/home/ayush/activities/reflect/utils.py", line 41, in <module>
    from jarabe import config
ModuleNotFoundError: No module named 'jarabe'

@chimosky
Copy link
Member

chimosky commented Mar 17, 2020

@ayushnawal I'm guessing you're using sugar live build for your development environment, if yes; run this

mv /usr/lib/python3.6/site-packages/jarabe /usr/lib/python3.6/dist-packages/

You should be able to start the activity after that.

@quozl
Copy link
Contributor

quozl commented Mar 18, 2020

ModuleNotFoundError: No module named 'jarabe'

It means the Python 3.6 interpreter you are using has no module jarabe installed. In turn because you have not installed the Sugar module for Python 3.6.

Temporarily, for the purpose of testing before you are finished, you can do something like what @chimosky said (corrected for the installed python version), or you can clone the Sugar module and put a copy of src/jarabe into your activity directory.

We have a policy of activities not doing import jarabe, so if there is a way to avoid that import, please do so. Find out why the activity is importing, and rewrite the code so that it does not. Many activities did import before the toolkit grew to include functions that were needed. So find the function in the toolkit and use it. We'll wait until you've removed the import, or proven that it must stay.

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.

Did a quick diff review. Found a minor change necessary. @ayushnawal did you checkout @Abhay-dot's Pull Request and the Pull Request linked to that? Thanks

@@ -713,7 +713,7 @@ def _choose_activity(self):
bundle_icons = utils.get_bundle_icons()
x = 0
y = 1
for bundle_id in bundle_icons.keys():
for bundle_id in list(bundle_icons.keys()):
Copy link
Member

@srevinsaju srevinsaju Apr 8, 2020

Choose a reason for hiding this comment

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

It would be better to remove list()

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.

4 participants