PEP8 in VDSM code

I suggest having pep8 a must for patch submission in VDSM.
http://www.python.org/dev/peps/pep-0008/

Currently there are a few people policing these rules in reviews but I suggest we make it automatic.

Unless someone objects I will put a gerrit hook that complains about pep8 violations.
It will not mark -1s until all (or at least most) source code has been converted because people might get complains about code they did not modify in this patch.

If you happy and you know it +1!

[Dan Kenigsberg]

On Thu, Mar 22, 2012 at 04:18:10PM -0400, Saggi Mizrahi wrote:
> I suggest having pep8 a must for patch submission in VDSM.
> http://www.python.org/dev/peps/pep-0008/
>
> Currently there are a few people policing these rules in reviews but I suggest we make it automatic.
>
> Unless someone objects I will put a gerrit hook that complains about pep8 violations.
> It will not mark -1s until all (or at least most) source code has been converted because people might get complains about code they did not modify in this patch.
>
> If you happy and you know it +1!

/me likie
+1

----- Original Message -----
> On Thu, Mar 22, 2012 at 04:18:10PM -0400, Saggi Mizrahi wrote:
> > I suggest having pep8 a must for patch submission in VDSM.
> > http://www.python.org/dev/peps/pep-0008/
> >
> > Currently there are a few people policing these rules in reviews
> > but I suggest we make it automatic.
> >
> > Unless someone objects I will put a gerrit hook that complains
> > about pep8 violations.
> > It will not mark -1s until all (or at least most) source code has
> > been converted because people might get complains about code they
> > did not modify in this patch.
> >
> > If you happy and you know it +1!
>
> /me likie
> +1

+1
> vdsm-devel mailing list
> [e-mail]
> https://fedorahosted.org/mailman/listinfo/vdsm-devel

On 03/22/2012 10:18 PM, Saggi Mizrahi wrote:
> I suggest having pep8 a must for patch submission in VDSM.
> http://www.python.org/dev/peps/pep-0008/
>
> Currently there are a few people policing these rules in reviews but I suggest we make it automatic.
>
> Unless someone objects I will put a gerrit hook that complains about pep8 violations.
> It will not mark -1s until all (or at least most) source code has been converted because people might get complains about code they did not modify in this patch.
>
> If you happy and you know it +1!

I'd rather avoid gerrit hooks if possible to use a jenkins job to 
validate this to keep the gerrit deployment as simple to 
maintain/upgrade as possible.

----- Original Message -----
> On 03/22/2012 10:18 PM, Saggi Mizrahi wrote:
> > I suggest having pep8 a must for patch submission in VDSM.
> > http://www.python.org/dev/peps/pep-0008/
> >
> > Currently there are a few people policing these rules in reviews
> > but I suggest we make it automatic.
> >
> > Unless someone objects I will put a gerrit hook that complains
> > about pep8 violations.
> > It will not mark -1s until all (or at least most) source code has
> > been converted because people might get complains about code they
> > did not modify in this patch.
> >
> > If you happy and you know it +1!
>
> I'd rather avoid gerrit hooks if possible to use a jenkins job to
> validate this to keep the gerrit deployment as simple to
> maintain/upgrade as possible.

But that's the wrong place to be doing it.
Jenkins periodically polls for changes and then runs a job and posts the results somewhere (who would get the email?)

Here the committer would immediately know that there is a problem with the patch and reviewers also immediately know not to accept it.
> vdsm-devel mailing list
> [e-mail]
> https://fedorahosted.org/mailman/listinfo/vdsm-devel

On Mon, Mar 26, 2012 at 04:57:24AM -0400, Ayal Baron wrote:
> > I'd rather avoid gerrit hooks if possible to use a jenkins job to
> > validate this to keep the gerrit deployment as simple to
> > maintain/upgrade as possible.
>
> But that's the wrong place to be doing it.
> Jenkins periodically polls for changes and then runs a job and posts
> the results somewhere (who would get the email?)
>
> Here the committer would immediately know that there is a problem with
> the patch and reviewers also immediately know not to accept it.

I think what Itamar is getting at is that from gerrit you can trigger
jenkins jobs which give a -1 if it fails. If jenkins checks for pep8
you've solved the feedback issue without creating custom a gerrit hook.
It will also be more scalable since you can add pyflakes / pylint / ...
in the same check.

On 03/26/2012 11:26 AM, Ewoud Kohl van Wijngaarden wrote:
> On Mon, Mar 26, 2012 at 04:57:24AM -0400, Ayal Baron wrote:
> > > I'd rather avoid gerrit hooks if possible to use a jenkins job to
> > > validate this to keep the gerrit deployment as simple to
> > > maintain/upgrade as possible.
> >
> > But that's the wrong place to be doing it.
> > Jenkins periodically polls for changes and then runs a job and posts
> > the results somewhere (who would get the email?)
> >
> > Here the committer would immediately know that there is a problem with
> > the patch and reviewers also immediately know not to accept it.
>
> I think what Itamar is getting at is that from gerrit you can trigger
> jenkins jobs which give a -1 if it fails. If jenkins checks for pep8
> you've solved the feedback issue without creating custom a gerrit hook.
> It will also be more scalable since you can add pyflakes / pylint / ...
> in the same check.

true.
per ayal's question - patch owner and reviewers will get the email, like 
any other review.
we need to keep the gerrit as simple as possible wrt maintenance.

The reason I wanted a gerrit hook is to avoid putting a -1 until VDSM is clean of errors.
It's supposed to be a transitional state.

----- Original Message -----
> From: "Itamar Heim" 
> To: "Ewoud Kohl van Wijngaarden" 
> Cc: [e-mail]
> Sent: Monday, March 26, 2012 5:52:13 AM
> Subject: Re: [vdsm] PEP8 in VDSM code
>
> On 03/26/2012 11:26 AM, Ewoud Kohl van Wijngaarden wrote:
> > On Mon, Mar 26, 2012 at 04:57:24AM -0400, Ayal Baron wrote:
> > > > I'd rather avoid gerrit hooks if possible to use a jenkins job to
> > > > validate this to keep the gerrit deployment as simple to
> > > > maintain/upgrade as possible.
> > >
> > > But that's the wrong place to be doing it.
> > > Jenkins periodically polls for changes and then runs a job and
> > > posts
> > > the results somewhere (who would get the email?)
> > >
> > > Here the committer would immediately know that there is a problem
> > > with
> > > the patch and reviewers also immediately know not to accept it.
> >
> > I think what Itamar is getting at is that from gerrit you can
> > trigger
> > jenkins jobs which give a -1 if it fails. If jenkins checks for
> > pep8
> > you've solved the feedback issue without creating custom a gerrit
> > hook.
> > It will also be more scalable since you can add pyflakes / pylint /
> > ...
> > in the same check.
>
> true.
> per ayal's question - patch owner and reviewers will get the email,
> like
> any other review.
> we need to keep the gerrit as simple as possible wrt maintenance.
> _______________________________________________
> vdsm-devel mailing list
> [e-mail]
> https://fedorahosted.org/mailman/listinfo/vdsm-devel