cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Animesh Chaturvedi <animesh.chaturv...@citrix.com>
Subject RE: Findbugs report on 4.3-forward
Date Wed, 29 Jan 2014 00:06:34 GMT
If I look at this commit for example

https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;a=commitdiff;h=92b4f66d73562e4211d2d787554ff229dbeb5705

It removes the two lines from LdapContextFactory.java

environment.put("com.sun.jndi.ldap.read.timeout", "500");-              
environment.put("com.sun.jndi.ldap.connect.pool", "true");

Is that reported by find bug? I don’t know this code  so not sure if it is intentional or
not ?

The point is there may be unintended risks in allowing late changes. 



-----Original Message-----
From: Animesh Chaturvedi [mailto:animesh.chaturvedi@citrix.com] 
Sent: Tuesday, January 28, 2014 3:35 PM
To: dev@cloudstack.apache.org
Subject: RE: Findbugs report on 4.3-forward

Are you sure all of the ones are needed. A quick look at 20+ commits from Daan  show many
formatting changes that may not be necessary and hinder quick review.

-----Original Message-----
From: Hugo Trippaers [mailto:trippie@gmail.com] 
Sent: Tuesday, January 28, 2014 3:16 PM
To: dev@cloudstack.apache.org
Subject: Re: Findbugs report on 4.3-forward



Sent from my iPhone

> On 28 jan. 2014, at 23:50, Animesh Chaturvedi <animesh.chaturvedi@citrix.com> wrote:
> 
> 
> -----Original Message-----
> From: Hugo Trippaers [mailto:trippie@gmail.com]
> Sent: Tuesday, January 28, 2014 2:37 PM
> To: dev@cloudstack.apache.org
> Cc: dev@cloudstack.apache.org
> Subject: Re: Findbugs report on 4.3-forward
> 
> Hey Animesh,
> 
> Not in agreement here. These are squashed bugs and we want as less bugs in the release
as possible.
> [Animesh] I understand but once we enter RC phase we only limit important fixes. I have
pulled in  2 commits from yours and 1 from Daan. 

We limited our fixes to only the important issues that we found. The other 6000 issues between
coverity and findbugs are still being triaged and will probably not make it into this release.

> 
> This is why we test any RC before we release it.
> [Animesh] Of course but timing is a bit off, if this was done a month back it would have
been fine.
> I say include all the big fixes we have in the release. If that means more testing before
we cut the RC then that is what it is. I can't rightfully vote for a release with known issues
with existing fixes. 
> [Animesh] Any release will have known issues, if we have fixes but can't be sure on regression
impact then we have to make a choice. 

Agreed, but we just don't agree on what that choice should be yet ;-) 

> Quality over release schedule would be my vote then.
> [Animesh] But why so late? Why was this activity not planned early on? I have been reminding
community to call out issues early on since like mid-December. 

On November 4 I sent the mail to the dev list that static code analysis (coverity) found 6000+
issues that needed to be triaged. I worked on quite a few with my colleagues, but it's a big
task for just the four of us. Findbugs just helped us to quickly identify the real scary issues
among them.
So I agree that the timing is less than ideal, but we should do our utmost best to ship the
highest quality piece of software we can. 

> 
> Cheers,
> Hugo
> 
> Sent from my iPhone
> 
>> On 28 jan. 2014, at 18:48, Animesh Chaturvedi <animesh.chaturvedi@citrix.com>
wrote:
>> 
>> Folks these issues reported by find-bugs have existed for some time. I am not confident
in picking them up now for 4.3 as it may break code that assumed old way of working. We can
take them up for 4.3 maintenance release. I wish we had done this exercise and not waited
until now.
>> 
>> I will pick Hugo's commit for which he called -1.
>> 
>> Thanks
>> Animesh
>> 
>> -----Original Message-----
>> From: Trippie [mailto:trippie@gmail.com] On Behalf Of Hugo Trippaers
>> Sent: Tuesday, January 28, 2014 1:29 AM
>> To: dev
>> Subject: Re: Findbugs report on 4.3-forward
>> 
>> Hey Animesh,
>> 
>> I agree with Daan here. We focussed on the bugs with a findbugs annotation of scariest.
I think that would warrant them to be included in the 4.3 release, so please cherry-pick them
all.
>> 
>> 
>> Cheers,
>> 
>> Hugo
>> 
>>> On 28 jan. 2014, at 09:32, Daan Hoogland <daan.hoogland@gmail.com> wrote:
>>> 
>>> Animesh,
>>> 
>>> I took up a lot of messages from findbugs in the server package over 
>>> the weekend. Not that I will attach my soul to the shipping of my 
>>> fixes but some of them are == vs eq and some are really nasty 
>>> nullpointer issues (a chack after first use is very common). You can 
>>> cherry-pick them or not. I don't think you should leave any of them 
>>> behind. Even with David being right we should go against him at 
>>> every convenient time, running the risk of being called his wife.
>>> 
>>>> On Tue, Jan 28, 2014 at 6:25 AM, Ian Duffy <ian@ianduffy.ie> wrote:
>>>> Hi Animesh,
>>>> 
>>>> Can you cherry-pick the below commit from from 4.3-forward to 4.3 branch?
>>>> 
>>>> Fix findbug issues within LDAP authenticator commit
>>>> 92b4f66d73562e4211d2d787554ff229dbeb5705
>>>> 
>>>> Thanks,
>>>> Ian
>>>> 
>>>> On 28 January 2014 03:48, Animesh Chaturvedi
>>>> <animesh.chaturvedi@citrix.com>wrote:
>>>> 
>>>>> Hugo I was reviewing your commits to 4.3-forward and looked at 
>>>>> your commits
>>>>> 
>>>>> 
>>>>> https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;a=commit;
>>>>> h = f18c5a1910b6370585a1d61638b8310c3ecba5ef
>>>>> 
>>>>> https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;a=commit;
>>>>> h = 60ac12780bfa1604902a89d5dc7937a8b9334e0d
>>>>> I think you want the last one which has fixes for NetUtils and 
>>>>> XenServerStorageMotionStrategy for which you had put -1 in first 
>>>>> RC but the commit includes more files. Can you make limited 
>>>>> changes directly to 4.3? I want to build another RC later tonight
>>>>> 
>>>>> Animesh
>>>>> 
>>>>> 
>>>>> -----Original Message-----
>>>>> From: Animesh Chaturvedi [mailto:animesh.chaturvedi@citrix.com]
>>>>> Sent: Monday, January 27, 2014 1:30 PM
>>>>> To: dev@cloudstack.apache.org
>>>>> Subject: RE: Findbugs report on 4.3-forward
>>>>> 
>>>>> Agreed
>>>>> 
>>>>> We need to fix the most important ones for 4.3. There may be 
>>>>> assumptions in the code which we may not know and may get broken 
>>>>> if these issues are fixed late. I will pull in the one Hugo casted 
>>>>> his
>>>>> -1 for the first vote, any others?
>>>>> 
>>>>> Animesh
>>>>> 
>>>>> -----Original Message-----
>>>>> From: David Nalley [mailto:david@gnsa.us]
>>>>> Sent: Monday, January 27, 2014 11:46 AM
>>>>> To: dev@cloudstack.apache.org
>>>>> Subject: Re: Findbugs report on 4.3-forward
>>>>> 
>>>>> So just curious if I am the only one concerned about a ton of 
>>>>> fixes going in at the last minute. If the fixes are for serious 
>>>>> bugs and we have consensus around their severity being high enough, indeed
lets fix things.
>>>>> My concern is that much of the QA we do is manual; and while we 
>>>>> are getting better; fixing tons of things at the last minute may 
>>>>> have unintended consequences that we don't know about and won't easily
find.
>>>>> 
>>>>> I yearn for the day when our automated testing is broad enough 
>>>>> that we can do fixes right up to the wire and know that things 
>>>>> still work, I am just not sure that I have confidence that we are there
yet.
>>>>> Thoughts? I am being paranoid?
>>>>> 
>>>>> --David
>>>>> 
>>>>> On Mon, Jan 27, 2014 at 3:11 AM, Daan Hoogland 
>>>>> <daan.hoogland@gmail.com>
>>>>> wrote:
>>>>>> Animesh, I commented the once i made yesterday with findbugs:
>>>>>> 
>>>>>> I allready send a few and will get you a list of the rest later today.
>>>>>> 
>>>>>> regards,
>>>>>> 
>>>>>> On Mon, Jan 27, 2014 at 3:48 AM, Animesh Chaturvedi 
>>>>>> <animesh.chaturvedi@citrix.com> wrote:
>>>>>>> Good job fellas. I see a number of commits 20+ into 4.3-forward
branch.
>>>>> Are their specific commits you want me to pick up out of these?
>>>>>>> 
>>>>>>> Animesh
>>>>>>> 
>>>>>>> -----Original Message-----
>>>>>>> From: Daan Hoogland [mailto:daan.hoogland@gmail.com]
>>>>>>> Sent: Sunday, January 26, 2014 2:41 AM
>>>>>>> To: dev
>>>>>>> Subject: Re: Findbugs report on 4.3-forward
>>>>>>> 
>>>>>>> I didn't get very far last night and will be looking at the 
>>>>>>> server
>>>>> package again this afternoon.
>>>>>>> 
>>>>>>> bon appétit,
>>>>>>> 
>>>>>>>> On Sun, Jan 26, 2014 at 1:36 AM, Ian Duffy <ian@ianduffy.ie>
wrote:
>>>>>>>> Hi,
>>>>>>>> 
>>>>>>>> Fixed the issues highlighted in the ldap user authentication
package.
>>>>>>>> 
>>>>>>>> Have pushed to 4.3-forward.
>>>>>>>> 
>>>>>>>> Thanks,
>>>>>>>> Ian
>>>>>>>> 
>>>>>>>> 
>>>>>>>> On 25 January 2014 22:26, Daan Hoogland 
>>>>>>>> <daan.hoogland@gmail.com>
>>>>> wrote:
>>>>>>>> 
>>>>>>>>>> or reply to this mail with the filename you are working
on
>>>>>>>>> I'll be looking at the server package as it seems to
contain 
>>>>>>>>> the most issues.
>>>>>>>>> 
>>>>>>>>> On Sat, Jan 25, 2014 at 4:00 PM, Hugo Trippaers 
>>>>>>>>> <hugo@trippaers.nl>
>>>>> wrote:
>>>>>>>>>> I've also added a job to master with the Findbugs
report and 
>>>>>>>>>> the
>>>>>>>>> cobertura code coverage report.
>>>>>>>>>> 
>>>>>>>>>> Good stuff, we have a 12% coverage of our classes
with unit tests.
>>>>>>>>>> Huge
>>>>>>>>> improvement over the last release where we had 4% iirc.
We 
>>>>>>>>> have
>>>>>>>>> 306 reports from Findbugs, of which the majority are

>>>>>>>>> internationalization
>>>>> issues.
>>>>>>>>> (String.getBytes without charset mostly). On the coverity
site 
>>>>>>>>> we have
>>>>>>>>> 6000+ issues still open, but at least that number is

>>>>>>>>> 6000+ relatively stable, we
>>>>>>>>> fix as much issues as we introduce and it's untuned so
we can 
>>>>>>>>> assume a large number of false positives there.
>>>>>>>>>> 
>>>>>>>>>> I think that on average the automated tools tell
us that code 
>>>>>>>>>> quality is
>>>>>>>>> improving, which a good thing. Combined with the functional

>>>>>>>>> testing and the simulator build we can prove that we
are doing 
>>>>>>>>> quite well on the code quality angle.
>>>>>>>>>> 
>>>>>>>>>> http://jenkins.buildacloud.org/job/build-master-slowbuild/
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>> Cheers,
>>>>>>>>>> 
>>>>>>>>>> Hugo
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>> On 25 jan. 2014, at 14:13, Daan Hoogland 
>>>>>>>>>> <daan.hoogland@gmail.com>
>>>>>>>>> wrote:
>>>>>>>>>> 
>>>>>>>>>>> H Hugo,
>>>>>>>>>>> 
>>>>>>>>>>> I'll spend some time on it tonight. Do you have
a work load 
>>>>>>>>>>> distribution scheme or is it random access?
>>>>>>>>>>> ;)
>>>>>>>>>>> 
>>>>>>>>>>> regards
>>>>>>>>>>> 
>>>>>>>>>>> On Sat, Jan 25, 2014 at 12:39 PM, Hugo Trippaers

>>>>>>>>>>> <hugo@trippaers.nl>
>>>>>>>>> wrote:
>>>>>>>>>>>> Hey all,
>>>>>>>>>>>> 
>>>>>>>>>>>> I've made Jenkins run the findbugs analysis
on 4.3-forward. 
>>>>>>>>>>>> Is there
>>>>>>>>> somebody who is willing to help triage the findings?
Maybe 
>>>>>>>>> there is some stuff that we need to fix?
>>>>>>>>>>>> 
>>>>>>>>>>>> the url is
>>>>>>>>> http://jenkins.buildacloud.org/job/cloudstack-4.3-forward-mave
>>>>>>>>> n
>>>>>>>>> -
>>>>>>>>> bui
>>>>>>>>> ld
>>>>>>>>> /3/findbugsResult/
>>>>>>>>>>>> 
>>>>>>>>>>>> Cheers,
>>>>>>>>>>>> 
>>>>>>>>>>>> Hugo
>> 
Mime
View raw message