On Wed, 2009-11-11 at 13:43 -0500, Chris Lumens wrote:
> kickstart.py never should have been written the way it previously was - with
> all the code that actually did something in the parse methods. This
> organization prevented the parse methods from being called until we had things
> like Storage instances and led to the multiple pass hack.
>
> This better design moves all the code that does anything into apply methods
> (that pykickstart lacks, oh well) and keeps the parse methods only setting
> values in KickstartCommand/KickstartData objects. This should allow doing
> parsing very early, then calling the apply methods when we're set up and
> therefore remove the reason for multiple passes.
>
> This patch requires a pykickstart which can pass data objects up from deep
> in dispatcher. Note also that this patch does not yet call any of the apply
> methods, so kickstart is temporarily busted.
Overall, this looks like a great improvement. I have one comment, below.
> ---
> kickstart.py | 747 +++++++++++++++++++++++++++-------------------------------
> 1 files changed, 347 insertions(+), 400 deletions(-)
>
> diff --git a/kickstart.py b/kickstart.py
> index 556f2cb..f1d7dcc 100644
> --- a/kickstart.py
> +++ b/kickstart.py
> @@ -1099,6 +1009,31 @@ class AnacondaKSHandler(superclass):
> self.id = self.anaconda.id
> self.onPart = {}
>
> + # All the KickstartCommand and KickstartData objects that
> + # handleCommand returns, so we can later iterate over them and run
> + # the execute methods. These really should be stored in the order
> + # they're seen in the kickstart file.
> + self._dataObjs = []
> +
> + def add(self, obj):
> + if isinstance(obj, KickstartCommand):
> + # Commands can only be run once, and the latest one seen takes
> + # precedence over any earlier ones.
> + i = 0
> + while i < len(self._dataObjs):
> + if self._dataObjs[i].__class__ == obj.__class__:
> + break
> +
> + i += 1
> +
> + if i == len(self._dataObjs):
> + self._dataObjs.append(obj)
> + else:
> + self._dataObjs[i] = obj
Shouldn't this instead remove the now-obsolete earlier command from the
list and then append the new one to the end of the list? As it stands,
you're not really preserving the ordering.
Dave
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
