ofbiz-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Nicolas Malin <nicolas.ma...@nereide.fr>
Subject Re: svn commit: r1817750 - in /ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/ base/component: ComponentConfig.java ComponentLoaderConfig.java ComponentResourceHandler.java
Date Mon, 11 Dec 2017 12:34:52 GMT
Same feel from France center :)

Nicolas


Le 11/12/2017 à 11:50, Taher Alkhateeb a écrit :
> On a side note to Michael kudos on the massive bug hunting effort lately.
>
> On Dec 11, 2017 1:33 PM, "Michael Brohl" <michael.brohl@ecomify.de> wrote:
>
>> I will see how I can handle this effectively without doing all my review
>> work twice.
>>
>> There should be not too much commits which are affected.
>>
>> I'll see if I can repair it this evening.
>>
>>
>> Am 11.12.17 um 11:29 schrieb Jacques Le Roux:
>>
>>> Michael,
>>>
>>> It would be easier for reviewers if you could revert your commits (not
>>> those of the weekend, I have not reviewed them all yet but most of them,
>>> and so far did not find any important issues, just few improvements I did).
>>> This would prevent us from double reviewing. For me at least r1817748 and
>>> 1817742 1817743 1817744 (the bigger ones ;))
>>>
>>> Thanks
>>>
>>> Jacques
>>>
>>>
>>> Le 11/12/2017 à 10:21, Michael Brohl a écrit :
>>>
>>>> These are good points, I wasn't aware of this and just saw the (in my
>>>> view) unnecessary extra lines of code.
>>>>
>>>> I will check my latest commits and change this back where it is
>>>> reasonable (i.e. where we have concatenations).
>>>>
>>>> Luckily, I just started this this morning and not during the heavier
>>>> commits over the weekend.
>>>>
>>>> This reminds me that it is NOT a good idea to do such changes along with
>>>> the commits of patches, it's too confusing.
>>>>
>>>> Thanks,
>>>>
>>>> Michael
>>>>
>>>>
>>>> Am 11.12.17 um 09:55 schrieb Jacopo Cappellato:
>>>>
>>>>> On Mon, Dec 11, 2017 at 9:31 AM, Jacques Le Roux <
>>>>> jacques.le.roux@les7arts.com> wrote:
>>>>>
>>>>> [...]
>>>>>> Also for OFBIZ-9872 the Jira lacks a description to possibly understand
>>>>>> why theses changes (notably why you rightly removed the "if
>>>>>> (Debug.verboseOn())" test because it's done at the bottom level with
>>>>>> "if
>>>>>> (isOn(level)) {"
>>>>>>
>>>>>> There is actually a valid reason for maintaining the pattern:
>>>>> if (Debug.verboseOn()) {
>>>>>       Debug.logVerbose(string + concatenation + here);
>>>>> }
>>>>>
>>>>> In fact string concatenation (in any of its forms) adds some overhead
>>>>> (computational, memory, garbage collection); since the string
>>>>> concatenation
>>>>> is done before calling Debug.logVerbose (or similar methods), but it
is
>>>>> only useful if that log level is on (otherwise the call to logVerbose
>>>>> will
>>>>> not produce any output) then it is wise to perform the verboseOn() check
>>>>> before: if the call returns a false then at runtime the sting
>>>>> concatenation
>>>>> will not be executed at all.
>>>>>
>>>>> Jacopo
>>>>>
>>>>>
>>>>
>>


Mime
View raw message