search

sugarlabs / sugar-toolkit-gtk3

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

Update toggletoolbutton.py #300

Closed Hrishi1999 closed 8 years ago

Hrishi1999 commented 8 years ago

Thanks. I will improve it. :D

Hrishi1999 commented 8 years ago

Done!

Hrishi1999 commented 8 years ago

Sorry for multiple commits. Please check last one.

Hrishi1999 commented 8 years ago

Pep8 checked.

quozl commented 8 years ago

Have not reviewed; a brief look shows many of the same mistakes made in #283 are being made again.

Hrishi1999 commented 8 years ago

@samdroid-apps @quozl @goutamnair7 Please review.

quozl commented 8 years ago
Hrishi1999 commented 8 years ago

Can you be more specific about last point.

quozl commented 8 years ago

I'm loath to be specific, because you respond by blindly making edits. I'm filled with regret afterwards.

One of my jobs 24 years ago was code review in an ISO-9001 quality system, and in that system our reviews were not allowed to present a solution to any review problem, but rather to describe the problem. This lets the author (you) use his best judgement in how to solve the review problem.

You should review the terminology, or terms, you have used in the text you added in toggletoolbutton.py.

You should also review the documentation for Gtk.ToggleToolButton to see what terms are used, and use those terms where appropriate. This is because the reader of your documentation will also be a reader of Gtk+ documentation.

If you are looking for a toggle button to experiment with, start Sugar, switch to the Journal, and the star on the top line is an instance of the Sugar ToggleToolButton. Interact with it. No accelerator has been set for this instance. See sugar:src/jarabe/journal/journaltoolbox.py for how it is made.

Hrishi1999 commented 8 years ago

Sorry for too much edits. I will take care of it from future.

Hrishi1999 commented 8 years ago

Fixed.

quozl commented 8 years ago

Reviewed bcb74a9 only. You added documentation to a class for a constructor using a subclass of the class. It will mislead the reader. You copied text from GTK2 documentation, yet Sugar is using GTK3. It will also mislead.

Hrishi1999 commented 8 years ago

Ok, I will fixed it again.

goutamnair7 commented 8 years ago

Your PR includes changes to progressicon.py as well. Please get that fixed - submit a different PR for different tasks.

Hrishi1999 commented 8 years ago

I dont know where it came from.

quozl commented 7 years ago

@Hrishi1999, welcome back. Will you be resubmitting these changes? It was sad for us to spend so much effort in this PR last year but not end up merging anything.

Hrishi1999 commented 7 years ago

@quozl It is done.

quozl commented 7 years ago

@Hrishi1999, thanks, this pull request with 34 commits over 2 files is still our all-time-record for ratio of number of commits in a pull request to number of files edited. :grin:

Hrishi1999 commented 7 years ago

@quozl, you want see that now.