On 10/04/2013 07:14 PM, Brian C. Lane wrote:
Does the order matter? If not, I'd rather see this added to self._required_pkgs at some point, either in preInstall or maybe as a default and then change preInstall to append to the list.
self._required_pkgs only exists to store the packages passed to preInstall() until install() is called. It is a minor flaw of the Payload interface itself that these two methods are not one since they are always called in succession (see install.py:161), but the DNFPayload has to respect that and the most straightforward way by far to pass the list on to nstall() is to store it in self._required_pkgs. That's the only reason why this instance variable exists and it might go away in the future. In no way would appending to it or even merging it with a new special instance variable, say _internally_required_pkgs result in a cleaner design and better maintainability than the single call to _install_package('dnf'), which is what I stress the most right after functionality.
I also notice that some of the variable naming is different from yumpayload.py
For example, dnf is using:
self._required_groups = [] self._required_pkgs = []
In Python, using underscore at the instance member start is used to indicate internal use in the instance. It does not really matter to the outisde world (like yumpayload.py) how such variable is called, it is part of the implementation details. Following the same name of a similar variable of different interface implementation can only be useful if the interface guaranteed its existence, which is hard to see for a private variable, or alternatively to breach the implementation encapsulation by the interface clients, which is definitely not desirable in this case, if ever.
Second, it can not be denied Anaconda is using both camelCase and underscore_convention in the code. Even the YumPayload itself is guilty of this:
self._repos_dir = "/etc/yum.repos.d,/etc/anaconda.repos.d,/tmp/updates/anaconda.repos.d,/tmp/product/anaconda.repos.d" ... self._requiredPackages = []
If Anaconda settled for one I would have no problem to follow the convention. Until then I decided to use underscores as I perceive it as more common in the current Python ecosystem.
Third, YumPayload will eventually get obsoleted and removed, so it makes little sense to be making the new module adhere to the old one in any way except the public interface.
It would be nice if we could use the same names where the functionality is the same between the packaging modules.
I perceive your comments to this trivial patch as somewhat bully-ish. Notice also that because of the time gap between us I will not be able to push this at least the following 24 hours. This is not really helping as the process of developing for Anaconda for (now) an outsider is already quite difficult as it is.
Ales