sugarlabs / sugar-toolkit-gtk3

Sugar Learning Environment, Activity Toolkit, GTK 3.
GNU Lesser General Public License v2.1
21 stars 80 forks source link

Port to Six #382

Closed quozl closed 4 years ago

quozl commented 6 years ago

Port the Sugar Toolkit for GTK+ 3 to Six, providing both Python 2 and Python 3 bindings from this package.

Progress goals:

this is an issue for tracking a major project which is larger than one person's contributions, so developers should make pull requests to satisfy any of these items, and not ask to own the issue.

quozl commented 6 years ago

A discussion began in pull-request https://github.com/sugarlabs/sugar-toolkit-gtk3/pull/385 about redundant work. As your release manager, my opinion is;

I'm unwilling to merge any pull requests until the whole issue is resolved; as I don't want master branch in a broken state in any point of git history, because this breaks use of "git bisect".

quozl commented 6 years ago

Reviewed the critical dependency during a meeting. It is probably a coding error; if PopWindow is used by an activity, the reference counted variable would not be shared with the shell, because the shell is in a different process. The shell is the only consumer of PopWindow. No activities use PopWindow. So the code can be removed from the toolkit and reimplemented in src/jarabe/desktop/activitychooser.py ActivityChooser.

Aniket21mathur commented 5 years ago

[ ] port the Hello World activity,

Ported by @pro-panda .

Aniket21mathur commented 5 years ago
* make local Fedora and Debian packages for testing by others.

Link to Debian packages (Python 2 version). https://github.com/Aniket21mathur/sugar-toolkit-gtk3-packages

Aniket21mathur commented 5 years ago
* port toolkit code in `src/sugar3` which calls C code, and test that the C code can be called from REPL,

@pro-panda @quozl I have not found any .so files in the respective directory. Also did a manual search, no import of any .c file in a python file.

Aniket21mathur commented 5 years ago
* regenerate documentation,

I wanted to know which documentation is referred here, and how do we regenerate it?

Aniket21mathur commented 5 years ago
* port graphics examples, and test that examples work,

Already ported by @pro-panda. I reviewed the changes again. :-)

rhl-bthr commented 5 years ago
* regenerate documentation,

I wanted to know which documentation is referred here, and how do we regenerate it?

We generate documentation using Sphinx. It is currently hosted at https://developer.sugarlabs.org/sugar3/

You can use https://github.com/sugarlabs/sugar-toolkit-gtk3/blob/master/make-doc.sh to generate documentation. It is generated from docstrings like this and this. I briefly reviewed https://github.com/sugarlabs/sugar-toolkit-gtk3/commit/aa8a5e70c415e6c2acf4ff373d9b366ac4692bb1 and did not see any changes in the docs. I've not reviewed this in other commits yet.

rhl-bthr commented 5 years ago
* port toolkit code in `src/sugar3` which calls C code, and test that the C code can be called from REPL,

@pro-panda @quozl I have not found any .so files in the respective directory. Also did a manual search, no import of any .c file in a python file.

Thanks, python does not support importing .c files directly. You must compile them and generate a .so to use it. We had one case in sugar-datastore.

I agree, there are no such cases in sugar-toolkit

rhl-bthr commented 5 years ago
* port graphics examples, and test that examples work,

Already ported by @pro-panda. I reviewed the changes again. :-)

Thanks

Aniket21mathur commented 5 years ago

I briefly reviewed aa8a5e7 and did not see any changes in the docs.

Thanks Agreed. I also looked for changes in https://github.com/sugarlabs/sugar-toolkit-gtk3/tree/master/doc, didn't find any, anywhere else to look?

rhl-bthr commented 5 years ago

I also looked for changes in https://github.com/sugarlabs/sugar-toolkit-gtk3/tree/master/doc, didn't find any, anywhere else to look?

Sorry, what did you look for ? You can go through each commit relating to the port to Python 3, which has been or will be pushed to master, and see if the changes made require changing the docs as well. If yes, please do.

Rephrasing my previous statement,

I briefly reviewed aa8a5e7 and did not see any changes in the docs.

I briefly reviewed aa8a5e7 and did not see any instances where the docs need to be updated

quozl commented 5 years ago

Generated docs are for the API, therefore look for changes to API. I don't remember any that were necessary for the port to Six, but the port to TelepathyGLib may cause some change. I'm falling behind while travelling, so this is from memory.

Aniket21mathur commented 5 years ago

@quozl @pro-panda thanks for explaining. Comapred changes in https://github.com/sugarlabs/sugar-toolkit-gtk3/pull/412/files with sugar3.activity.activity module , sugar3.presence.activity module, sugar3.presence.buddy module, sugar3.presence.connectionmanager module, sugar3.presence.presenceservice module, sugar3.presence.sugartubeconn module, sugar3.presence.tubeconn module API docs. The telepathy port does not include any changes in function names mentioned in the API docs, nor does it contradicts their description. I see no edits to the current documentation, but their might be something that we can add, like how to use telepathy_text_chan in activities? Need suggestions.

quozl commented 5 years ago

Reviewed changes so far; we've completed documentation changes; of the changes to API that were made, the documentation did not originally describe what was changed. Tracking the opportunity in another issue.

sukhdeepg commented 5 years ago

@Aniket21mathur as you quoted:

As the toolkit supports both Python and Python 3 we should have multi-version packages for the toolkit

I had a question: I checked the python code in the repository, for the py files I saw, it was python3. So, how are we providing python2 support? and to completely understand the package meaning, is it what we install using apt(in case of Ubuntu) or the zip file which is created after make dist. Thanks in advance.

rhl-bthr commented 5 years ago

Hi @sukhdeepg,

this is not the best place to ask doubts. Please use the mailing list.

I checked the python code in the repository, for the py files I saw, it was python3

Which files did you see ? A .py file does not enforce the Python version with which it is supposed to be executed. I'm assuming you confused it with shebang line.

...and to completely understand the package meaning, is it what we install using apt(in case of Ubuntu) or the zip file which is created after make dist

Yes, you usually install packages using apt, and use the make dist command in the process of generating commands. I've skipped a lot of details on how these work. Feel free to explore

quozl commented 5 years ago

Component sugar-toolkit-gtk3 branch master is both Python 2 and Python 3 compatible right now. This was achieved using Six. Both autogen.sh and configure accept an argument to select the version of Python to build for.

Hiding subthread as resolved. Better forum is mailing list. But I don't think you had exhausted all sources of information before asking your question. :grin:

sukhdeepg commented 5 years ago

@pro-panda @quozl my bad for not researching thoroughly. Point taken and understood 🙂

quozl commented 5 years ago

No worries. I saw you subscribed to the mailing list. Thanks!