Let's add some DASDs ....again (#1070115)
Vratislav Podzimek
vpodzime at redhat.com
Wed Sep 3 12:00:03 UTC 2014
On Tue, 2014-09-02 at 08:39 -0400, Samantha N. Bueno wrote:
> Contains all suggestions from prior review, minus bcl's one suggestion
> for blivet patches, which I later realized isn't needed:
>
> > > + if dev is None or dev == "":
> > > + raise ValueError("You have not specified a device number or the number is invalid")
> > > + dev = dev.lower()
> > > + bus = dev[:string.rfind(dev, ".") + 1]
> > > + dev = dev[string.rfind(dev, ".") + 1:]
> >
> > I'd pull the rfind out into its own variable so you can check for -1
> > and raise an error (assuming no . is an error)
>
> Lack of '.' is not necessarily an error. There's now a docstring which
> notes valid formats for the device number, but basically either 0.0.abcd
> or abcd (or caps are also ok if a-f appear in the dev number).
Apart from the minor comments these all look good to me. Consider the
comments and suggestions and 'fixing-locally' stuff. ACK.
--
Vratislav Podzimek
Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic
More information about the anaconda-patches
mailing list