[PATCH 2/2] DNFPayload: install DNF itself.
Ales Kozumplik
akozumpl at redhat.com
Mon Oct 7 08:38:50 UTC 2013
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
More information about the anaconda-patches
mailing list