[PATCH 15/16] Add support for KS %addon section and the API+code to use it
Martin Sivak
msivak at redhat.com
Thu Dec 13 16:59:35 UTC 2012
> > + 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?
> > + 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
> > 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.
> > + 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).
More information about the anaconda-patches
mailing list