On Mon, 2014-03-24 at 08:08 -0400, Anne Mulhern wrote:
----- Original Message -----
> From: "David Lehman" <dlehman(a)redhat.com>
> To: "anaconda patch review"
<anaconda-patches(a)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