Re: [Freeipa-devel] [PATCHES 0442-0449] Pylint: sunday code cleanup
On 21.03.2016 16:52, Petr Spacek wrote: On 21.3.2016 13:50, Lukas Slebodnik wrote: On (21/03/16 12:30), Martin Basti wrote: On 21.03.2016 10:33, Christian Heimes wrote: On 2016-03-21 10:29, Petr Spacek wrote: On 20.3.2016 21:56, Martin Basti wrote: Patches attached. I do not really like freeipa-mbasti-0442-pylint-remove-bare-except because it replaces most of try: ... except: with try: ... except Exception: which AFAIK does not add any value. It would be better to replace Exception with more specific exception so the code raises an error instead of continuing when something really unexpected happens. It adds some value. A bare except also excepts signals like KeyboardInterrupt and SystemExit. except Exception doesn't block these exceptions. But yes, more specific exceptions are better. Christian 'except Exception' is another pylint check :D I replaced bare except with a particular exception in cases where it was clear. For other occurrences of bare except it covers too much Exception types, so catch Exception is more sensible, or I need crystal ball to detect what kind of exceptions can be raised there. Agree. It can be changed to more specific exceptions type of Exception in future. This change is less risky. pylint passed on fedora {23, 24, rawhide} ACK Okay, it makes sense. master: * 491447cc5ab8c5eff2be57d609201cefb79f7053 pylint: remove bare except * e93e89e1ae27e4f0ef23001f6c1247c45695ae24 Pylint: fix definition of global variables * 5add0f94cf9253a72224ccaf5be38540468ea589 Pylint: enable pointless-except check * d46cd5d956d1c03b863bf90d0fd0ff4870448183 Pylint: enable reimported check * 195e50b93b63e4f30ce83dbcfef278727d48aea2 Pylint: use list comprehension instead of iteration * b66028af1815fbf7666b82ebeaa81ad56996a74f Pylint: import max one module per line * da0318d4d7dd369be136449e686b6fb46d0cc5d8 Pylint: remove unnecessary-semicolon * 4a396dd68b1bc6cc68765f502f7e952a087064a8 Pylint: enable invalid-name check -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCHES 0442-0449] Pylint: sunday code cleanup
On 21.3.2016 13:50, Lukas Slebodnik wrote: > On (21/03/16 12:30), Martin Basti wrote: >> On 21.03.2016 10:33, Christian Heimes wrote: >>> On 2016-03-21 10:29, Petr Spacek wrote: On 20.3.2016 21:56, Martin Basti wrote: > Patches attached. I do not really like freeipa-mbasti-0442-pylint-remove-bare-except because it replaces most of try: ... except: with try: ... except Exception: which AFAIK does not add any value. It would be better to replace Exception with more specific exception so the code raises an error instead of continuing when something really unexpected happens. >>> It adds some value. A bare except also excepts signals like >>> KeyboardInterrupt and SystemExit. except Exception doesn't block these >>> exceptions. >>> >>> But yes, more specific exceptions are better. >>> >>> Christian >>> >>> >>> >>> >> 'except Exception' is another pylint check :D >> >> I replaced bare except with a particular exception in cases where it was >> clear. For other occurrences of bare except it covers too much Exception >> types, so catch Exception is more sensible, or I need crystal ball to detect >> what kind of exceptions can be raised there. >> > Agree. > > It can be changed to more specific exceptions type of Exception in future. > This change is less risky. > > pylint passed on fedora {23, 24, rawhide} > > ACK Okay, it makes sense. -- Petr^2 Spacek -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCHES 0442-0449] Pylint: sunday code cleanup
On (21/03/16 12:30), Martin Basti wrote: >On 21.03.2016 10:33, Christian Heimes wrote: >>On 2016-03-21 10:29, Petr Spacek wrote: >>>On 20.3.2016 21:56, Martin Basti wrote: Patches attached. >>>I do not really like >>>freeipa-mbasti-0442-pylint-remove-bare-except >>>because it replaces most of >>> >>>try: ... except: >>> >>>with >>> >>>try: ... except Exception: >>> >>> >>>which AFAIK does not add any value. It would be better to replace Exception >>>with more specific exception so the code raises an error instead of >>>continuing >>>when something really unexpected happens. >>It adds some value. A bare except also excepts signals like >>KeyboardInterrupt and SystemExit. except Exception doesn't block these >>exceptions. >> >>But yes, more specific exceptions are better. >> >>Christian >> >> >> >> >'except Exception' is another pylint check :D > >I replaced bare except with a particular exception in cases where it was >clear. For other occurrences of bare except it covers too much Exception >types, so catch Exception is more sensible, or I need crystal ball to detect >what kind of exceptions can be raised there. > Agree. It can be changed to more specific exceptions type of Exception in future. This change is less risky. pylint passed on fedora {23, 24, rawhide} ACK LS -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCHES 0442-0449] Pylint: sunday code cleanup
On 21.03.2016 10:33, Christian Heimes wrote: On 2016-03-21 10:29, Petr Spacek wrote: On 20.3.2016 21:56, Martin Basti wrote: Patches attached. I do not really like freeipa-mbasti-0442-pylint-remove-bare-except because it replaces most of try: ... except: with try: ... except Exception: which AFAIK does not add any value. It would be better to replace Exception with more specific exception so the code raises an error instead of continuing when something really unexpected happens. It adds some value. A bare except also excepts signals like KeyboardInterrupt and SystemExit. except Exception doesn't block these exceptions. But yes, more specific exceptions are better. Christian 'except Exception' is another pylint check :D I replaced bare except with a particular exception in cases where it was clear. For other occurrences of bare except it covers too much Exception types, so catch Exception is more sensible, or I need crystal ball to detect what kind of exceptions can be raised there. Martin^2 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCHES 0442-0449] Pylint: sunday code cleanup
On 2016-03-21 10:29, Petr Spacek wrote: > On 20.3.2016 21:56, Martin Basti wrote: >> Patches attached. > > I do not really like > freeipa-mbasti-0442-pylint-remove-bare-except > because it replaces most of > > try: ... except: > > with > > try: ... except Exception: > > > which AFAIK does not add any value. It would be better to replace Exception > with more specific exception so the code raises an error instead of continuing > when something really unexpected happens. It adds some value. A bare except also excepts signals like KeyboardInterrupt and SystemExit. except Exception doesn't block these exceptions. But yes, more specific exceptions are better. Christian signature.asc Description: OpenPGP digital signature -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCHES 0442-0449] Pylint: sunday code cleanup
On 20.3.2016 21:56, Martin Basti wrote: > Patches attached. I do not really like freeipa-mbasti-0442-pylint-remove-bare-except because it replaces most of try: ... except: with try: ... except Exception: which AFAIK does not add any value. It would be better to replace Exception with more specific exception so the code raises an error instead of continuing when something really unexpected happens. Other patches look sensible to me. -- Petr^2 Spacek -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code