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. >> >> >>
