Recently, we've had a bugs http://gerrit.ovirt.org/#/c/24242/ that could have been avoided had we used pylint to check our code before its usage. Two other bugs-in-waiting can be quickly removed http://gerrit.ovirt.org/#/c/24313/ http://gerrit.ovirt.org/#/c/24412/
I would like to suggest http://gerrit.ovirt.org/#/c/24382/ "make check: add a pylint check"
We have a long way to go before pylint is happy with our code, but I believe that pylinting some of our modules is a good start.
The down sides are many: it's slow, it's another dependency, it has false negatives, and I do not yet understand how it behaves (particularly, the interdependency between checked modules).
What do you think? Should we add it?
Regards, Dan.
----- Original Message -----
From: "Dan Kenigsberg" danken@redhat.com To: "vdsm-devel" vdsm-devel@lists.fedorahosted.org Cc: lbednar@redhat.com Sent: Wednesday, February 12, 2014 11:25:57 PM Subject: [vdsm] pylint in make check-local
Recently, we've had a bugs http://gerrit.ovirt.org/#/c/24242/ that could have been avoided had we used pylint to check our code before its usage. Two other bugs-in-waiting can be quickly removed http://gerrit.ovirt.org/#/c/24313/ http://gerrit.ovirt.org/#/c/24412/
I would like to suggest http://gerrit.ovirt.org/#/c/24382/ "make check: add a pylint check"
We have a long way to go before pylint is happy with our code, but I believe that pylinting some of our modules is a good start.
The down sides are many: it's slow, it's another dependency, it has false negatives, and I do not yet understand how it behaves (particularly, the interdependency between checked modules).
What do you think? Should we add it?
+1
My take: I am for it, or at very least to give it a try, following the path you suggested.
I use pylint in a few other projects and it saved me from some bugs and led to improvements. IMO it is worth its price.
On Thu, Feb 13, 2014 at 03:00:21AM -0500, Francesco Romani wrote:
Recently, we've had a bugs http://gerrit.ovirt.org/#/c/24242/ that could have been avoided had we used pylint to check our code before its usage. Two other bugs-in-waiting can be quickly removed http://gerrit.ovirt.org/#/c/24313/ http://gerrit.ovirt.org/#/c/24412/
I would like to suggest http://gerrit.ovirt.org/#/c/24382/ "make check: add a pylint check"
We have a long way to go before pylint is happy with our code, but I believe that pylinting some of our modules is a good start.
The down sides are many: it's slow, it's another dependency, it has false negatives, and I do not yet understand how it behaves (particularly, the interdependency between checked modules).
What do you think? Should we add it?
+1
My take: I am for it, or at very least to give it a try, following the path you suggested.
I use pylint in a few other projects and it saved me from some bugs and led to improvements. IMO it is worth its price.
While I'm waiting for more acks/nacks, could infra add "pylint" to our Jenkins slaves?
Regarding Nir's on-gerrit comment: I'd like to have pylint run by default in order to avoid adding new pylint-detectable bugs. If we do not add a pylint_blacklist now, I do not believe we'd ever make it empty.
On Mon, Feb 17, 2014 at 11:44:37AM +0000, Dan Kenigsberg wrote:
While I'm waiting for more acks/nacks, could infra add "pylint" to our Jenkins slaves?
Is the version in EL6 recent enough or do we have to ship our own version? (EL6 is 0.21.1)
Please review http://gerrit.ovirt.org/24556
On Mon, Feb 17, 2014 at 01:21:11PM +0100, Ewoud Kohl van Wijngaarden wrote:
On Mon, Feb 17, 2014 at 11:44:37AM +0000, Dan Kenigsberg wrote:
While I'm waiting for more acks/nacks, could infra add "pylint" to our Jenkins slaves?
Is the version in EL6 recent enough or do we have to ship our own version? (EL6 is 0.21.1)
Please review http://gerrit.ovirt.org/24556
Let's take the standard one, and move to something more modern (such as Fedora's 1.0.0) when the need arrises.
On Mon, Feb 17, 2014 at 01:11:41PM +0000, Dan Kenigsberg wrote:
On Mon, Feb 17, 2014 at 01:21:11PM +0100, Ewoud Kohl van Wijngaarden wrote:
On Mon, Feb 17, 2014 at 11:44:37AM +0000, Dan Kenigsberg wrote:
While I'm waiting for more acks/nacks, could infra add "pylint" to our Jenkins slaves?
Is the version in EL6 recent enough or do we have to ship our own version? (EL6 is 0.21.1)
Please review http://gerrit.ovirt.org/24556
Let's take the standard one, and move to something more modern (such as Fedora's 1.0.0) when the need arrises.
Sounds good. Patch merged and somewhere during the next 30 minutes it should be deployed on all slaves.
vdsm-devel@lists.fedorahosted.org