Linux Archive

Linux Archive (http://www.linux-archive.org/)
-   ArchLinux Pacman Development (http://www.linux-archive.org/archlinux-pacman-development/)
-   -   Clean up and simplify some things in the custom storage spoke. (http://www.linux-archive.org/archlinux-pacman-development/674211-clean-up-simplify-some-things-custom-storage-spoke.html)

Chris Lumens 06-18-2012 06:25 PM

Clean up and simplify some things in the custom storage spoke.
 
> @@ -417,28 +421,16 @@ class CustomPartitioningSpoke(NormalSpoke):
>
> # Make sure there's something displayed on the RHS. Just default to
> # the first mountpoint in the page.
> + # FIXME: the current page appears to be the default/empty/create page,
> + # even if you've obviously gone into one of the roots to remove
> + # a device
> page = self._accordion.currentPage()
> - if not page or not page._members:
> - return
> -
> - self._populate_right_side(page._members[0])
> + if getattr(page, "_members", []):
> + self._populate_right_side(page._members[0])

This is a kind of weird phrasing. Why don't you use hasattr instead?

> @@ -513,10 +499,13 @@ class CustomPartitioningSpoke(NormalSpoke):
> # Devices just created by autopartitioning will be listed as unused
> # since they are not yet a part of any known Root.
> for device in self._unusedDevices():
> + if device.exists:
> + continue
> +
> if device.format.type == "swap":
> swaps.append(device)
>
> - if hasattr(device.format, "mountpoint"):
> + if getattr(device.format, "mountpoint", None):
> mounts[device.format.mountpoint] = device

Likewise.

- Chris

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

David Lehman 06-18-2012 06:45 PM

Clean up and simplify some things in the custom storage spoke.
 
On Mon, 2012-06-18 at 14:25 -0400, Chris Lumens wrote:
> > @@ -417,28 +421,16 @@ class CustomPartitioningSpoke(NormalSpoke):
> >
> > # Make sure there's something displayed on the RHS. Just default to
> > # the first mountpoint in the page.
> > + # FIXME: the current page appears to be the default/empty/create page,
> > + # even if you've obviously gone into one of the roots to remove
> > + # a device
> > page = self._accordion.currentPage()
> > - if not page or not page._members:
> > - return
> > -
> > - self._populate_right_side(page._members[0])
> > + if getattr(page, "_members", []):
> > + self._populate_right_side(page._members[0])
>
> This is a kind of weird phrasing. Why don't you use hasattr instead?

This also catches the case where it has the attribute but the value is
not set to anything useful. This instance may be an unnecessary hack. It
was added to deal with CreateNewPage instances not having a _members
attribute.

>
> > @@ -513,10 +499,13 @@ class CustomPartitioningSpoke(NormalSpoke):
> > # Devices just created by autopartitioning will be listed as unused
> > # since they are not yet a part of any known Root.
> > for device in self._unusedDevices():
> > + if device.exists:
> > + continue
> > +
> > if device.format.type == "swap":
> > swaps.append(device)
> >
> > - if hasattr(device.format, "mountpoint"):
> > + if getattr(device.format, "mountpoint", None):
> > mounts[device.format.mountpoint] = device

This one is useful in checking for a mountpoint attr that is actually
set to something as opposed to just checking for the attr.

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


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

Chris Lumens 06-18-2012 06:55 PM

Clean up and simplify some things in the custom storage spoke.
 
> > This is a kind of weird phrasing. Why don't you use hasattr instead?
>
> This also catches the case where it has the attribute but the value is
> not set to anything useful. This instance may be an unnecessary hack. It
> was added to deal with CreateNewPage instances not having a _members
> attribute.

self._members is added by Page.__init__, which CreateNewPage derives
from. So there should always be a self._members. However it may very
well be an empty list. That was also sloppy on my part.

- Chris

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

David Lehman 06-18-2012 07:06 PM

Clean up and simplify some things in the custom storage spoke.
 
On Mon, 2012-06-18 at 14:55 -0400, Chris Lumens wrote:
> > > This is a kind of weird phrasing. Why don't you use hasattr instead?
> >
> > This also catches the case where it has the attribute but the value is
> > not set to anything useful. This instance may be an unnecessary hack. It
> > was added to deal with CreateNewPage instances not having a _members
> > attribute.
>
> self._members is added by Page.__init__, which CreateNewPage derives
> from. So there should always be a self._members. However it may very
> well be an empty list. That was also sloppy on my part.

CreateNewPage doesn't ever call the Page constructor AFAICT. Maybe
that's the correct fix.

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


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

Chris Lumens 06-18-2012 07:16 PM

Clean up and simplify some things in the custom storage spoke.
 
> CreateNewPage doesn't ever call the Page constructor AFAICT. Maybe
> that's the correct fix.

Oh, I completely glossed over that.

The reason CreateNewPage doesn't call the Page constructor is because I
don't want it to get the Data and System labels.

So yeah, it's entirely possible there will be no self._members.

- Chris

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


All times are GMT. The time now is 03:38 AM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.