Re: [openstack-dev] [neutron] Which changes need accompanying bugs?

2014-08-14 Thread Andreas Jaeger
On 08/14/2014 11:37 AM, Ihar Hrachyshka wrote: On 14/08/14 02:43, Angus Lees wrote: On Wed, 13 Aug 2014 11:11:51 AM Kevin Benton wrote: Is the pylint static analysis that caught that error prone to false positives? If not, I agree that it would be really nice if that were made part of the tox

Re: [openstack-dev] [neutron] Which changes need accompanying bugs?

2014-08-14 Thread Maru Newby
On Aug 13, 2014, at 11:11 AM, Kevin Benton blak...@gmail.com wrote: Is the pylint static analysis that caught that error prone to false positives? If not, I agree that it would be really nice if that were made part of the tox check so these don't have to be fixed after the fact. To me that

[openstack-dev] [neutron] Which changes need accompanying bugs?

2014-08-13 Thread Angus Lees
I'm doing various small cleanup changes as I explore the neutron codebase. Some of these cleanups are to fix actual bugs discovered in the code. Almost all of them are tiny and obviously correct. A recurring reviewer comment is that the change should have had an accompanying bug report and

Re: [openstack-dev] [neutron] Which changes need accompanying bugs?

2014-08-13 Thread Kevin Benton
I'm not sure what the guideline is, but I would like to point out a good reason to have the bug report even for obvious fixes. When users encounters bugs, they go to launchpad to report them. They don't first scan the commits of the master branch to see what was fixed. Having the bug in launchpad

Re: [openstack-dev] [neutron] Which changes need accompanying bugs?

2014-08-13 Thread Ihar Hrachyshka
-BEGIN PGP SIGNED MESSAGE- Hash: SHA512 On 13/08/14 09:28, Angus Lees wrote: I'm doing various small cleanup changes as I explore the neutron codebase. Some of these cleanups are to fix actual bugs discovered in the code. Almost all of them are tiny and obviously correct. A

Re: [openstack-dev] [neutron] Which changes need accompanying bugs?

2014-08-13 Thread Armando M.
I am gonna add more color to this story by posting my replies on review [1]: Hi Angus, You touched on a number of points. Let me try to give you an answer to all of them. (I'll create a bug report too. I still haven't worked out which class of changes need an accompanying bug report and which

Re: [openstack-dev] [neutron] Which changes need accompanying bugs?

2014-08-13 Thread Kevin Benton
Is the pylint static analysis that caught that error prone to false positives? If not, I agree that it would be really nice if that were made part of the tox check so these don't have to be fixed after the fact. To me that particular patch seems like one that should be accompanied with a unit

Re: [openstack-dev] [neutron] Which changes need accompanying bugs?

2014-08-13 Thread Angus Lees
On Wed, 13 Aug 2014 11:11:51 AM Kevin Benton wrote: Is the pylint static analysis that caught that error prone to false positives? If not, I agree that it would be really nice if that were made part of the tox check so these don't have to be fixed after the fact. At the moment pylint on

Re: [openstack-dev] [neutron] Which changes need accompanying bugs?

2014-08-13 Thread Angus Lees
On Wed, 13 Aug 2014 12:46:03 AM Kevin Benton wrote: I'm not sure what the guideline is, but I would like to point out a good reason to have the bug report even for obvious fixes. When users encounters bugs, they go to launchpad to report them. They don't first scan the commits of the master

Re: [openstack-dev] [neutron] Which changes need accompanying bugs?

2014-08-13 Thread Sumit Naiksatam
On Wed, Aug 13, 2014 at 5:43 PM, Angus Lees g...@inodes.org wrote: On Wed, 13 Aug 2014 11:11:51 AM Kevin Benton wrote: Is the pylint static analysis that caught that error prone to false positives? If not, I agree that it would be really nice if that were made part of the tox check so these

Re: [openstack-dev] [neutron] Which changes need accompanying bugs?

2014-08-13 Thread Armando M.
At the moment pylint on neutron is *very* noisy, and I've been looking through the reported issues by hand to get a feel for what's involved. Enabling pylint is a separate discussion that I'd like to have - in some other thread. I think enabling pylint check was discussed at the