cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Daan Hoogland <daan.hoogl...@gmail.com>
Subject Re: Findbugs report on 4.3-forward
Date Wed, 29 Jan 2014 12:02:22 GMT
come to think of it, I probably shouldn't burden you with the decision
on each of those commits. So if you don't want to go through all of
them I will review them and compile a list. If you don't want to wait
my call would be include all.

On Wed, Jan 29, 2014 at 12:48 PM, Daan Hoogland <daan.hoogland@gmail.com> wrote:
> Animesh,
>
> As for my fixes, I agree with you that not all of them are vital. You
> pulled only one and this seems too few. There where more null pointers
> and uses off  == and more scary things. I have rerouted my efforts to
> master now. Again (Hugo end I don't always agree on practical matters,
> on principals we usually do) I think it is up to you which ones to
> pull. I rember I removed unused local vars. This doesn't seem vital
> (or dangerous) but pulling only one seems very few.
>
>
> On Wed, Jan 29, 2014 at 7:11 AM, Animesh Chaturvedi
> <animesh.chaturvedi@citrix.com> wrote:
>>
>>
>> -----Original Message-----
>> From: Ian Duffy [mailto:ian@ianduffy.ie]
>> Sent: Tuesday, January 28, 2014 6:34 PM
>> To: CloudStack Dev
>> Subject: RE: Findbugs report on 4.3-forward
>>
>> Hi Animesh,
>>
>> Tested all those changes to detail. Those lines were removed due to unexpected behavior
that I had not spotted until now.
>> [Animesh] That's what my worry is there may be unintended changes. I suspect this
one was not from find bugs. Looking at others from Daan there are many formatting changes
which are not necessary and hinder review
>>
>> They were suppose to allow for better fall over between multiple domain controllers,
how ever they were causing caching to occur. This meant if a users password was reset in LDAP
the old password was still allowing login for a limited time.
>>
>> Please pull the changes forward,
>> Thanks
>>
>> Ian.
>> On 29 Jan 2014 00:07, "Animesh Chaturvedi" <animesh.chaturvedi@citrix.com>
>> wrote:
>>
>>> 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.
>>>
>>>
>>>

Mime
View raw message