> + 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).