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