Re: [master][PATCH] Use the right index for selecting region (#1114234)

[Anne Mulhern]

----- Original Message -----
> From: "Vratislav Podzimek" 
> To: "anaconda patch review" 
> Sent: Tuesday, July 1, 2014 1:52:55 AM
> Subject: Re: [master][PATCH] Use the right index for selecting region	(#1114234)
>
> On Mon, 2014-06-30 at 17:48 -0400, David Shea wrote:
> > On 06/30/2014 05:16 PM, Chris Lumens wrote:
> > > > > diff --git a/pyanaconda/ui/tui/spokes/time_spoke.py
> > > > > b/pyanaconda/ui/tui/spokes/time_spoke.py
> > > > > index 62a1b23..3995a70 100644
> > > > > --- a/pyanaconda/ui/tui/spokes/time_spoke.py
> > > > > +++ b/pyanaconda/ui/tui/spokes/time_spoke.py
> > > > > @@ -104,7 +104,7 @@ class TimeZoneSpoke(FirstbootSpokeMixIn,
> > > > > NormalTUISpoke):
> > > > >                       self.apply()
> > > > >                       self.close()
> > > > >                   else:
> > > > >                self.app.switch_screen(self, self._regions[id])
> > > > >                self.app.switch_screen(self, self._regions[index])
> > > > >             return INPUT_PROCESSED
> > > > >         elif key.lower() == "b":
> > > > >             self.app.switch_screen(self, None)
> > > >
> > > > This should have been caught by pylint, yeah?
> > >
> > > After poking around at this a little bit... no, not really.  I can make
> > > "id" a bad variable name which shows that we are using it in a couple
> > > places, but doesn't do anything for this case.  I can make "id" a bad
> > > builtin function to use but that doesn't catch this either.  So we would
> > > either have to write something for this (which I'm not especially
> > > inclined to do) or just keep an eye out.
> > > Chris
> > >
> > > anaconda-patches mailing list
> > > [e-mail]
> > > https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
> >
> > Do you think it'd be something worth raising upstream? pylint doesn't
> > have checks on list index operations, so maybe it could use some. Here's
> > a quick dirty version as a pylint plugin:
> > http://fpaste.org/114472/16488614/
>
> I think this should definitely be done in the upstream code.
>
> --
> Vratislav Podzimek
>
> Anaconda Rider | RHCE | Red Hat, Inc. | Brno - Czech Republic
>
> anaconda-patches mailing list
> [e-mail]
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches

Agreed.

- mulhern