search

sugarlabs / showntell-activity

ShowNTell expands the capabilities of ClassroomPresenter to enable users to create presentations on the XO and to record audio descriptions of the slides.
GNU General Public License v2.0
0 stars 6 forks source link

Port to Python3 #16

Open Saumya-Mishra9129 opened 4 years ago

Saumya-Mishra9129 commented 4 years ago

@quozl @pro-panda Please review. Fixes #13

rhl-bthr commented 4 years ago

Reviewed. Not tested. Looks good to me

quozl commented 4 years ago

In https://github.com/sugarlabs/showntell-activity/pull/16/commits/c4a2255f8c5d413efd6a4abf8e36f9f414e7db40 and https://github.com/sugarlabs/showntell-activity/pull/16/commits/25d30082088c9c2e518f52a89f48d0af53400bea please properly clean up the code that is no longer needed. These are fragments for supporting very old Python, and with our port to Python 3 guide we do not need to support Python below 3.

Saumya-Mishra9129 commented 4 years ago

In c4a2255 and 25d3008 please properly clean up the code that is no longer needed. These are fragments for supporting very old Python, and with our port to Python 3 guide we do not need to support Python below 3.

@quozl I think we do not even need path.py as what we use in other activities is Path class from pathlib. The path.py file is of very old version of python (maybe 2.2).

quozl commented 4 years ago

You're probably right. It was new once. Perhaps a new feature of path was needed by the original authors of this activity.

Saumya-Mishra9129 commented 4 years ago

Since bea3301 is not related to Python 2 to 3, let's cherry-pick this in a separate branch and raise a new PR for this commit

Yeah done with changes!

Saumya-Mishra9129 commented 4 years ago

In c4a2255 and 25d3008 please properly clean up the code that is no longer needed. These are fragments for supporting very old Python, and with our port to Python 3 guide we do not need to support Python below 3.

@quozl It is done in 83f8ad1 . Please Review.

quozl commented 4 years ago

Thanks. I've reviewed your changes to https://github.com/sugarlabs/showntell-activity/pull/16/commits/83f8ad14baf6a16ae3a27ab592ddec4b058713b6. I'm not familiar with the activity; what testing have you done?

Saumya-Mishra9129 commented 4 years ago

Thanks. I've reviewed your changes to 83f8ad1. I'm not familiar with the activity; what testing have you done?

@quozl I am testing with https://wiki.sugarlabs.org/go/Activities/ShowNTell. I didn't find any other thing.

Saumya-Mishra9129 commented 4 years ago

@quozl Have you done testing as I mentioned here??

Thanks. I've reviewed your changes to 83f8ad1. I'm not familiar with the activity; what testing have you done?

@quozl I am testing with https://wiki.sugarlabs.org/go/Activities/ShowNTell. I didn't find any other thing.

quozl commented 4 years ago

No, I've not had time to do testing yet. Can you tell me what testing you did?