search

cginternals / libzeug

deprecated: C++ sanctuary for small but powerful and frequently required, stand alone features.
MIT License
16 stars 13 forks source link

Add OptionChanged-Signal to AbstractProperty #117

Closed lw0 closed 9 years ago

scheibel commented 9 years ago

Please merge the current libzeug master.

cgcostume commented 9 years ago

:+1: (please update, then merge and close), but wait for ok form @scheibel or @sbusch42

sbusch42 commented 9 years ago

Do not mix variables between methods. Please add a new public block at the top (for public variables) and put the signal optionChanged there. After that is done and the branch is updated, :shipit:

sbusch42 commented 9 years ago

Could we please stop removing comments? You never know what is obvious for new users, so please document wherever possible instead of avoiding documentation wherever possible. The documentation for the signal was fine, please restore and for the future the aim should be to document every item at least in the public interface.

scheibel commented 9 years ago

I had to read your comment about the variables and the according source code four times to recognize you're talking about attributes/members. I wondered where in this really small PR you saw strange placed temporary variables. Concept naming (especially in C++) is just strange...

sbusch42 commented 9 years ago

I was talking about the signal "optionChanged", which I clearly stated in my comment ... It is placed in between methods, which is not nice. Public variables (or attributes) deserve an own block.

scheibel commented 9 years ago

:shipit: ? :ship: !