On a side note to Michael kudos on the massive bug hunting effort lately.
On Dec 11, 2017 1:33 PM, "Michael Brohl" <michael.br...@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.r...@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
>>>>
>>>>
>>>
>>>
>>
>
>
