[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [patch] Several patches to process Dogtail testcases in Anaconda

On Fri, 2007-04-20 at 13:34 +0200, Alexander Todorov wrote:
> What's in the tar.gz:

FYI -- in the future, it's a little easier to review if patches are
attached directly rather than inside of archives where you have to open
more apps to read through them.  Comments are also a lot easier then.

> * anaconda.process_and_launch_dogtail_script.diff
> The biggest patch of all. This adds a --dogtail option to the command
> line options for anaconda. Checks all possible conditions specifying a
> dogtail script: kickstart, boot cmd line, anaconda cmd line option.
> Downloads the script, fork and execute it before anaconda.intf.run().

The boot command line should just lead to the option being passed on the
anaconda command line from the loader.  See loader2/loader.c's handling
of the various args like resolution, xdriver, etc.  It's also far more
normal to set the value of anaconda.dogtail to None instead of "" for
unset.  We'll also eventually need to have the filename not be
hard-coded as that opens things up to race conditions, especially as
anaconda starts being run on "live" systems in cases like the live CD

> * gui.py.always_load_a11y.diff
> Removed checking for dogtail. Always loads accessibility.

What's the reasoning for always loading?  Loading when needed instead is
a lot nicer as it helps to reduce the memory footprint in cases where
it's not needed.

> * installdata.py.save_dogtail_kickstart.diff
> Added code that writes the dogtail option to the /root/anaconda-ks.cfg
> after install is complete.

"" vs None again

> * scripts.upd-instroot.dogtail_files.diff
> Added code to extract the missing files into stage2.img.
> Works fine, already sent to anaconda-devel-list.

Why are the gconf bits needed here?  At least in the past, loading the
modules was good enough to get things initialized and that was all that
the gconf key affected.  ie, what's changed?

> * pykickstart.data.add_dogtail.diff
> * pykickstart.parser.add_dogtail_command.diff

Looks okay at a glance.

> Please review these patches.

Overall, looking good... I do want to hold off on applying them until
after we branch anaconda for F7 at this point just to avoid breaking
things in the final freeze.  But that shouldn't be a big deal.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]