[PATCH 6/9] Add action classes for container member set management.

Vratislav Podzimek vpodzime at redhat.com
Mon Mar 24 14:36:19 UTC 2014


On Mon, 2014-03-24 at 08:08 -0400, Anne Mulhern wrote:
> 
> 
> 
> ----- Original Message -----
> > From: "David Lehman" <dlehman at redhat.com>
> > To: "anaconda patch review" <anaconda-patches at lists.fedorahosted.org>
> > Sent: Friday, March 21, 2014 11:14:02 AM
> > Subject: Re: [PATCH 6/9] Add action classes for container member set	management.
> > 
> > On Fri, 2014-03-21 at 08:51 +0100, Vratislav Podzimek wrote:
> > > On Thu, 2014-03-20 at 12:14 -0500, David Lehman wrote:
> > > > ---
> > > >  blivet/deviceaction.py | 184
> > > >  ++++++++++++++++++++++++++++++++++++++++++++++---
> > > >  tests/action_test.py   |  96 ++++++++++++++++++++++++++
> > > >  2 files changed, 271 insertions(+), 9 deletions(-)
> > > > 
> > > > diff --git a/blivet/deviceaction.py b/blivet/deviceaction.py
> > > > index d9a6a87..569377a 100644
> > > > --- a/blivet/deviceaction.py
> > > > +++ b/blivet/deviceaction.py
> > > > @@ -1,8 +1,7 @@
> > > >  # deviceaction.py
> > > > -# Device modification action classes for anaconda's storage
> > > > configuration
> > > > -# module.
> > > > +# Device modification action classes.
> > > >  #
> > > > -# Copyright (C) 2009  Red Hat, Inc.
> > > > +# Copyright (C) 2009-2014  Red Hat, Inc.
> > > >  #
> > > >  # This copyrighted material is made available to anyone wishing to use,
> > > >  # modify, copy, or redistribute it subject to the terms and conditions
> > > >  of
> > > > @@ -49,19 +48,25 @@ ACTION_TYPE_NONE = 0
> > > >  ACTION_TYPE_DESTROY = 1000
> > > >  ACTION_TYPE_RESIZE = 500
> > > >  ACTION_TYPE_CREATE = 100
> > > > +ACTION_TYPE_ADD = 50
> > > > +ACTION_TYPE_REMOVE = 10
> > > >  
> > > >  action_strings = {ACTION_TYPE_NONE: "None",
> > > >                    ACTION_TYPE_DESTROY: "Destroy",
> > > >                    ACTION_TYPE_RESIZE: "Resize",
> > > > -                  ACTION_TYPE_CREATE: "Create"}
> > > > +                  ACTION_TYPE_CREATE: "Create",
> > > > +                  ACTION_TYPE_ADD: "Add",
> > > > +                  ACTION_TYPE_REMOVE: "Remove"}
> > > If we change those dicts to:
> > > { key1: val1,
> > >   key2: val2,
> > >   ...
> > >   keyN: valN,
> > > }
> > > 
> > > we will be able to add more items without ruining the git history of the
> > > last item. But that's just a neatpick.
> > 
> > I always disliked that syntax, but I hadn't thought of that reason to
> > like/use it.
> > 
> 
> I tend to prefer not associating the indentation of the keys so closely with the
> number of characters in the variable to which they are assigned.
> If you need to change action_strings to device_action_strings and you want to keep
> indentation that screws up the git history of every key value pair.
> 
> Which is better:
> 
> action_strings = { ACTION_TYPE_NONE: "None",
>    ACTION_TYPE_DESTROY: "Destroy",
>    ...etc.
> or
> 
> action_strings = \
>    { ACTION_TYPE_NONE: "None",
>      ACTION_TYPE_DESTROY: "Destroy",
>      ...etc.
> 
> ?
> 
> I've tended to do the first...but now I'm thinking that the second is better.
The second looks better to me as well.

-- 
Vratislav Podzimek

Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic



More information about the anaconda-patches mailing list