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

[Fix] flake8 errors #8

Closed chimosky closed 5 years ago

quozl commented 6 years ago

Rebase your branch to master first, thanks.

chimosky commented 6 years ago

@quozl kindly review.

quozl commented 6 years ago

GitHub says "This branch has conflicts". You haven't rebased to our master branch. Your faf82cf is after 436fce487b10972060e811c2036ebaf2c853ac9f when it really should be after c32fb04a6eee1acbfe1b7de15ba5370cc0acaa0a.

Your pull request is from your gstreamer1.0-port branch. Doing a "git diff" between your gstreamer1.0-port branch and our master branch shows only flake8 fixes; as if you've rebased from an older master commit.

I did spend half an hour reviewing your changes, by fetching your branch, rebasing to master, and making changes on top of it, but it does not seem worth pushing my changes.

In general;

(Also, given issue https://github.com/sugarlabs/showntell-activity/issues/2 is so fundamental a divergence, perhaps the divergence should be fixed as a priority. Making these flake8 changes causes some more divergence. The second commit in this repository is closest to version 2 of the activity, and there are versions 6 and 11 available on activities.sugarlabs.org.)

chimosky commented 6 years ago

Changes made

  • continue editing until the flake8 errors have fallen to zero; they are 307 with master branch, and 127 with your branch, so this is good progress, but not finished,

I'll do what i can.

  • you'll need to exclude E402, see the Chat activity file .flake8 for how to do this, and once you do the flake8 output will change,

Added file, thanks for the ref - was looking for it -.

  • make sure you are using python2 with flake8 for the moment; the activity is not yet ported to python3,

I am.

(Also, given issue #2 is so fundamental a divergence, perhaps the divergence should be fixed as a priority. Making these flake8 changes causes some more divergence. The second commit in this repository is closest to version 2 of the activity, and there are versions 6 and 11 available on activities.sugarlabs.org.)

Are you saying i should make a new release before this fix? and one after?.

quozl commented 6 years ago

Are you saying i should make a new release before this fix? and one after?.

Not at all. I'm saying the source code here is like a wandering child that has stepped off a path, and rather than encourage them to wander further, it would be better to bring them back to the path.

Here's a brief family history of this activity source code;

showntell

Tony had released v1 and v2 while updating gitorious.

Tony had released v6 and v11 without updating gitorious; this was quite normal, there was never a requirement to maintain source code in git, what matters is the bundle.

Cristian seems to have used gitorious to do a port to GTK+ 3, and so it began from v2 sources. Result was uploaded to GitHub.

Rahul and yourself have continued to make changes. The tail grows longer.

Issue #2 will consist of;

Putting it another way; the next version (v12) shouldn't be released until it follows on from the latest version (v11).

As the tail grows, so too does the size of the task to bring forward the v2 to v11 changes.

References:

chimosky commented 6 years ago

So v12 will contain everything, including commit history of v2 to v11?.

quozl commented 6 years ago

So v12 will contain everything, including commit history of v2 to v11?.

Thank you, that would be ideal.

rhl-bthr commented 5 years ago

Stale, closing