Re: Left over master branch pychecker warnings

[Jeremy Katz <katzj redhat com>]

On Wed, 2008-10-29 at 17:13 +0100, Hans de Goede wrote:
> Chris Lumens wrote:
> >> isys/isys.py:898: Function return types are inconsistent
> > 
> > If this were None vs. a dict then it'd be NOTABUG as far as I'm
> > concerned.  The addition of whatever devices_prop_iface is means we need
> > to ask dcantrell.
> > 
>  >> rescue.py:82: Function return types are inconsistent
>  >> text.py:461: Function return types are inconsistent
>  >
>  > I think the call to OkCancelWindow here results in the return value
>  > magically getting returned.  I know we do this sort of thing in the gui
>  > interface (check all the *Window methods in gui.py:InstallInterface and
>  > follow the tags).
>  >
> 
> Ok, so what do we do with these warnings, we can disable them completely using
> --no-returnvalues
> 
> Or filter them out on a file by file basis (The warning is to generic to be 
> able to filter per function).

Personally, I lean towards disabling completely unless we have a case
where it's actually found something useful.

Maybe we should have 'make strict-check' also to be looked over from
time to time that actually doesn't hide things like this ;)

> >> autopart.py:201: Using integer division (start / 1024) may return integer or float
> > 
> >> iw/lvm_dialog_gui.py:91: Using integer division (pesize / 1024) may 
> >> return integer or float
> > Probably worth fixing just to be absolutely correct.
>
> Ok, call me a python newbie (I am) so how do I fix those, by using a cast like 
> in C ?

Yeah

> >> installclass.py:60: Methods (createbug, getbug, getbugs, getversion, 
> >> login, query) in AbstractFiler need to be overridden in a subclass
> > 
> > This is by design.  The AbstractFiler's methods will never be called
> > since AbstractFiler.supportsFiling returns False.  NOTABUG.
> > 
> 
> So filter this case, or filter all warnings of this type using --no-abstract ?

I suspect we could filter all warnings of this type.  

> >> iscsi.py:319: string.atoi is deprecated
> >> iw/lvm_dialog_gui.py:282: string.atoi is deprecated
> > 
> > Again, worth fixing for correctness I suppose.
> > 
> Jeremy disagrees, so what will it be, I can easily disabel all deprecated 
> warnings using --no-deprecated

Oh, I'm fine with making the change.  I just don't know if we want
things to fail because of deprecated users :)

> >> iw/partition_ui_helpers_gui.py:51: Comparisons with True are not 
> >> necessary and may not work as expected
> > 
> > I've cleaned up a bunch of these pychecker warnings in the past so it's
> > worth fixing this one now.
> > 
> 
> So I guess the:
> "if rc == True:"
> Should become:
> "if rc:"

That seems to be the implication.  Although that then lets values other
than True be allow-able

> >> network.py:697: Using is not , may not always work
> > 
> > 'is not' should probably be '!=' here.
> > 
> 
> Shall I fix that then or should we wait for dcantrell?

Fix away!

Jeremy