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.hoogl...@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.chaturv...@citrix.com> wrote: >> >> >> -----Original Message----- >> From: Ian Duffy [mailto:i...@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.chaturv...@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. >>> >>> >>>
