[openstack-dev] [Ceilometer] Adding pylint checking of new ceilometer patches

Igor Degtiarov idegtiarov at mirantis.com
Mon Oct 6 09:38:21 UTC 2014


My points are next:

1. Pylint check will be very useful for project, and will help to
avoid critical errors or mistakes in code.

2. At first step we won't implement pylint as gate job, but will add
it at master to have a possibility to check  code with pylint locally,
if it is needed.

3. In future it could be added as a non-voting job.
-- Igor


On Sat, Oct 4, 2014 at 1:56 AM, Angus Lees <gus at inodes.org> wrote:
> You can turn off lots of the "refactor recommendation" checks.  I've been
> running pylint across neutron and it's uncovered half a dozen legitimate
> bugs so far - and that's with many tests still disabled.
>
> I agree that the defaults are too noisy, but its about the only tool that
> does linting across files - pep8 for example only looks at the current file
> (and not even the parse tree).
>
> On 4 Oct 2014 03:22, "Doug Hellmann" <doug at doughellmann.com> wrote:
>>
>>
>> On Oct 3, 2014, at 1:09 PM, Neal, Phil <phil.neal at hp.com> wrote:
>>
>> >> From: Dina Belova [mailto:dbelova at mirantis.com]
>> >> On Friday, October 03, 2014 2:53 AM
>> >>
>> >> Igor,
>> >>
>> >> Personally this idea looks really nice to me, as this will help to
>> >> avoid
>> >> strange code being merged and not found via reviewing process.
>> >>
>> >> Cheers,
>> >> Dina
>> >>
>> >> On Fri, Oct 3, 2014 at 12:40 PM, Igor Degtiarov
>> >> <idegtiarov at mirantis.com> wrote:
>> >> Hi folks!
>> >>
>> >> I try too guess do we need in ceilometer checking new patches for
>> >> critical errors with pylint?
>> >>
>> >> As far as I know Nova and Sahara and others have such check. Actually
>> >> it is not checking of all project but comparing of the number of
>> >> errors without new patch and with it, and if diff is more then 0 then
>> >> patch are not taken.
>> >
>> > Looking a bit deeper it seems that Nova struggled with false positives
>> > and resorted to https://review.openstack.org/#/c/28754/ , which layers some
>> > historical checking of git on top of pylint's tendency to check only the
>> > latest commit. I can't say I'm too deeply versed in the code,  but it's
>> > enough to make me wonder if we want to go that direction and avoid the
>> > issues altogether?
>>
>> I haven’t looked at it in a while, but I’ve never been particularly
>> excited by pylint. It’s extremely picky, encourages enforcing some
>> questionable rules (arbitrary limits on variable name length?), and repots a
>> lot of false positives. That combination tends to result in making writing
>> software annoying without helping with quality in any real way.
>>
>> Doug
>>
>> >
>> >>
>> >> I have taken as pattern Sahara's solution and proposed a patch for
>> >> ceilometer:
>> >> https://review.openstack.org/#/c/125906/
>> >>
>> >> Cheers,
>> >> Igor Degtiarov
>> >>
>> >> _______________________________________________
>> >> OpenStack-dev mailing list
>> >> OpenStack-dev at lists.openstack.org
>> >> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>> >>
>> >>
>> >>
>> >>
>> >> --
>> >> Best regards,
>> >> Dina Belova
>> >> Software Engineer
>> >> Mirantis Inc.
>> > _______________________________________________
>> > OpenStack-dev mailing list
>> > OpenStack-dev at lists.openstack.org
>> > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>
>>
>> _______________________________________________
>> OpenStack-dev mailing list
>> OpenStack-dev at lists.openstack.org
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
>
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>



More information about the OpenStack-dev mailing list