> I don't fully understand the reasoning behind this because the new
> features are backwards compatible. Anyway I've moved them into a separate
> class and updated the control map.
The reason you want a completely new class is because FC3-F10 did not
support this option. We don't want to recognize it as valid when we
parse kickstart versions for those releases. If someone's doing a RHEL5
install test and uses a kickstart file they just copied from their
rawhide system, we want to make sure we give them an error.
> I only don't understand what's the purpose of:
>
> removedKeywords = KickstartCommand.removedKeywords
> removedAttrs = KickstartCommand.removedAttrs
>
>
> in FC3_Upgrade (and other classes) and not sure if I need it in the new class.
James explained these already. While you don't need them since we've
never removed any attributes or options from the upgrade command, I like
to see them in each class so that all objects have these attributes.
That way the BaseCommand and BaseData objects can just assume they're
present.
> pykickstart/commands/upgrade.py | 33 +++++++++++++++++++++++++++++++++
> pykickstart/handlers/control.py | 4 ++--
> 2 files changed, 35 insertions(+), 2 deletions(-)
>
> diff --git a/pykickstart/commands/upgrade.py b/pykickstart/commands/upgrade.py
> index b7dba86..5fbf93a 100644
> --- a/pykickstart/commands/upgrade.py
> +++ b/pykickstart/commands/upgrade.py
> @@ -49,3 +49,36 @@ class FC3_Upgrade(KickstartCommand):
> self.upgrade = True
> else:
> self.upgrade = False
> +
> +class F11_Upgrade(FC3_Upgrade):
Please do add:
removedKeywords = FC3_Upgrade.removedKeywords
removedAttrs = FC3_Upgrade.removedAttrs
here as class attributes. Subclasses do not inherit class attributes of
their superclass (at least, they didn't in my testing) so we have to set
these explicitly.
> + def __init__(self, writePriority=0, *args, **kwargs):
> + FC3_Upgrade.__init__(self, writePriority, *args, **kwargs)
> +
> + self.op = self._getParser()
> + self.root_device = kwargs.get("root_device", None)
> +
> + def __str__(self):
> + if self.upgrade and (self.root_device is not None):
> + retval="# Upgrade existing installation\nupgrade --root-device=%s\n" % self.root_device
> + else:
> + retval=FC3_Upgrade.__str(self)__
You meant retval = FC3_Upgrade.__str__(self) here, I'm sure.
> +
> + return retval
> +
> + def _getParser(self):
> + op = KSOptionParser(lineno=self.lineno)
> + op.add_option("--root-device", dest="root_device")
> + return op
Instead of calling KSOptionParser, call FC3_Upgrade._getParser(self)
here to pull in whatever arguments FC3_Upgrade's KSOptionParser object
may support. I know it's none in this case, but that's how it works in
every other class in pykickstart and I really like things to be
consistent.
Everything else looks fine to me.
- Chris
_______________________________________________
Kickstart-list mailing list
Kickstart-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/kickstart-list
