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 16:47:05 GMT
The first five I allready posted. The ones between brackets seem minor
to me. Then again they also pose the least risk so in my opinion you
should pull them all. The last one is a bug we found during testing
(which was already fixed in master a while back).

these three in combination:
43ba36f97950aa8d09399a28bb50c6a22209f15e
f3529a19a9aad36dbd92e311018643629f19c748
437ff438af23f7ff499200e212966f275a309a75


9a1b882d0eb871c64fe0f0f3fbafbabae89188fa
(9aced41d708acd22b43ef0e512fa2cc657a6c0a2)
(b40156313adce56ae80a622e885ecd9ee403c2f6)
f3b9a7f1232e8f39e619bf99155a4e36ff2d88f9
c58e509924e97dfced80981da5c27b9f3aae9b33
(23a3d99fc0836839a88584d7aa6a978d99c5d126)
866a539b067663b346bd8d087c578987a86fe834
0b13f8e59db1e681e1dff0baa828bb0711842e9f
20127e09dc0e341a2f790a8a52cded4c5f1f0cc1
791b7f8f7fe8f7d342e46fbb61a3f421e87fca59
9772693dca9a91a438078de11b1be1e6044aa6f3
9495c68c1378e379880433d45ec43bfda75ec3f9

and

caf17c2f46bf317ec8966b08aaff728a73fda14a

On Wed, Jan 29, 2014 at 4:47 PM, Animesh Chaturvedi
<animesh.chaturvedi@citrix.com> wrote:
> I have asked for specific commits already a few times. Can you provide list today?
>
> Thanks
> Animesh
>
> On Jan 29, 2014, at 4:03 AM, "Daan Hoogland" <daan.hoogland@gmail.com> wrote:
>
>> 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