[PATCH 15/16] Add support for KS %addon section and the API+code to use it
Vratislav Podzimek
vpodzime at redhat.com
Thu Dec 13 17:10:50 UTC 2012
On Thu, 2012-12-13 at 11:59 -0500, Martin Sivak wrote:
> > > + def handle_line(self, line):
> > > + """Process one kickstart line."""
> > > + self.content += line
> > Does line end with the newline character?
>
> Hmm.. good question, I have no idea :)
>
>
> > > + if self.addon_id and not hasattr(self.handler.addon,
> > > self.addon_id):
> > > + setattr(self.handler.addon, self.addon_id,
> > > AnacondaKSAddon(self.addon_id))
> > Why are you using setattr here?
>
> Because self.addon_id (the id of the addon) is a string we got from the filesystem and I need to set an attribute with the same name?
Ah, of course, I somehow got confused. :)
>
> > > + with progress_report(_("Configuring addons")):
> > ^^^^ I guess we need a better word
> > here.
>
> Probably, but we will have real 3rd party addons too here, not just externally maintained components
I don't like 'Configuring', 'addon' is another story.
>
> > > topology = None
> > >
> > > +
> > > class AnacondaKSScript(KSScript):
>
> > No newline needed here and the same goes for the AnacondaKSHandler
> > class.
>
> That is actually PEP8 style.. two lines between classes. But I can remove it of course..
>
>
> > > os.path.basename(os.path.dirname(os.path.abspath(path)))
> > Are all os.path.* calls needed here? Why to get abspath when you use
> > dirname and basename afterwards?
>
> Because if any path contains "something/../spokes" you will get "something/.." out of dirname and ".." out of basename.
Ah, okay. Maybe there should be something like os.path.expandpath() for
these cases.
>
> > > + if not os.path.isdir(path):
> > > + continue
> > > +
> > > + classes = collect(module_name, path, lambda cls:
> > > issubclass(cls, self.AddonClassType))
> > > + print classes
> > I guess we don't need this print.
> >
> > > + if classes:
> > > + addons[addon_id] = classes[0](name = addon_id)
> > Could there be multiple classes? And if yes, how it is ensured that
> > the
> > right one is the first in the list?
>
> Only one matching class per plugin was the plan. But this is still open to changes. I need to see real use cases from other teams first (ID handling would have to change if we allow more classes).
Okay.
--
Vratislav Podzimek
Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic
More information about the anaconda-patches
mailing list