timlau / yumex-dnf-old

Next Generation of Yum Extender using DNF as backend
GNU General Public License v2.0
61 stars 17 forks source link

Many small changes to updater.py #122

Closed genodeftest closed 8 years ago

genodeftest commented 8 years ago

Some work on updater.py, probably perfectionistic ;)

Not ready to merge yet, there is still one issue with printing to the wrong command line (see my question on the mailing list. I'm just opening this here so that you know what's going on and – hopefully – don't merge a conflicting change to updater.py at the same time.

For reasons for those changes, please have a look at commit descriptions.

genodeftest commented 8 years ago

Now ready to merge in my opinion. @timlau?

timlau commented 8 years ago

Looks fine to me

The only minor issue is: Why the the double _ on local vars (ex. self.__somevars), normally a singe _ is used to mark somthing private. Double _ is normally only used for __init__ type of methods. I use a single _ private methods/vars in other parts of the code, so you should problerly also do this for consistancy.

EDIT Just checked the official docs, __ os used to protect a var/method from being overloaded in a child class, and _ is used to mark at method or var af non public API. As far as I can see there is no need to use __ in this code https://docs.python.org/3.4/tutorial/classes.html#tut-private

genodeftest commented 8 years ago

Yes, I understand __ (double underscore) as marking it "private" and _ (single underscore) as marking it protected to speak in Java terms. Since we're not using inheritation here, both have the same effect, so it should not matter. Right?

timlau commented 8 years ago

It does not matter from a functional point of view, just about consistancy feel free to merge it