-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 03/19/2010 04:05 AM, Hans de Goede wrote:

> +
> + import storage
> + if isinstance(d, storage.devices.NetworkStorageDevice):
> + s = args.append(self.anaconda.network.dracutSetupStrin g(d))
> + if s not in args:
> + args.append(s)

I think this bit should just be:

args.append(self.anaconda.network.dracutSetupStrin g(d))

args.append() doesn't return anything, so no need to assign it, and even
if it did, you just appended it to args

The rest looks good, although the core loop of the get method could
probably be rewritten as a " ".join()


What are your thoughts on imports inside of methods? I see the import
storage was there previously, but in general I like imports at the top
of the module so I can keep track of what gets used and what's already
available for the module as a whole. Python will only evaluate the
import once, but it seems inefficient to have it inside a loop like that.

- --
Brian C. Lane <bcl@redhat.com>
Red Hat / Port Orchard, WA
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)

iQEVAwUBS6Os7RF+jBaO/jp/AQL07Af8Dzyq/XZBjL7vXpxR8bmcZtxQDFrFbTy/
xloo1DZCSeHdIc3qeKPvj8wDhi5ZXHOtXpRRAsTKTNG/SFXWIbWLPNcExtgEmUxq
wAkSj704Kp+Cd4j6goijVBs2KUEznhyGqImQ4GExQQRVo8sPAn qB92GvQYiZnK9I
omgo+F8nEvvD40u2+MmCs7mMkjfm72gchpHjkkArFcIsu+y2Hl DRJaIJ6cHHNrDj
MRSWww19P/LTi3XI6yE7DcBvlpSMmf49ZwZBmFoUoAFjyRnk4LdDRGtwKh3k SYqO
dH+EYVJ7sJKaNqsI1i/wLiXi48+z4DI8jSIX2UlnLe4k3CcP1Y8ykA==
=sMrQ
-----END PGP SIGNATURE-----

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

Hi,

On 03/19/2010 05:57 PM, Brian C. Lane wrote:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 03/19/2010 04:05 AM, Hans de Goede wrote:


+
+ import storage
+ if isinstance(d, storage.devices.NetworkStorageDevice):
+ s = args.append(self.anaconda.network.dracutSetupStrin g(d))
+ if s not in args:
+ args.append(s)


I think this bit should just be:

args.append(self.anaconda.network.dracutSetupStrin g(d))



Oh, good catch, although what it should be is:

+ if isinstance(d, storage.devices.NetworkStorageDevice):
+ s = self.anaconda.network.dracutSetupString(d)
+ if s not in args:
+ args.append(s)

This way we won't get the dracut kernel cmdline arguments for bringing
up the NIC twice, if we somehow need 2 disks connected through the same
network adapter during early boot.



What are your thoughts on imports inside of methods? I see the import
storage was there previously, but in general I like imports at the top
of the module so I can keep track of what gets used and what's already
available for the module as a whole. Python will only evaluate the
import once, but it seems inefficient to have it inside a loop like that.



Normally I prefer imports at the top too, but iirc it was done this way
because otherwise things would not work due to an import loop.

Thanks for the review!

Regards,

Hans

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

Hi,

On 03/19/2010 05:57 PM, Brian C. Lane wrote:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 03/19/2010 04:05 AM, Hans de Goede wrote:


+
+ import storage
+ if isinstance(d, storage.devices.NetworkStorageDevice):
+ s = args.append(self.anaconda.network.dracutSetupStrin g(d))
+ if s not in args:
+ args.append(s)


I think this bit should just be:

args.append(self.anaconda.network.dracutSetupStrin g(d))



p.s.

This is fixed in my local tree now.

Regards,

Hans

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list