I've reverted these commits and reapplied the original patches.
Thanks all for taking care,
Michael
Am 11.12.17 um 15:21 schrieb Jacques Le Roux:
I reviewed r1817748 1817742 1817743 1817744 there are OK with me
So you just have to revert the removing of the tests on Debug in these
commits
I reviewed r1817748 1817742 1817743 1817744 there are OK with me
So you just have to revert the removing of the tests on Debug in these commits
(IIRW only in r1817748 and r1817750)
Notably beware of
"if (debug) {" in r1817748.
Jacques
Le 11/12/2017 à 12:39, Michael Brohl a écrit :
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" wrote:
I will see how I can handle this effectively without
Thanks, Taher! :-)
Am 11.12.17 um 11:50 schrieb Taher Alkhateeb:
On a side note to Michael kudos on the massive bug hunting effort lately.
On Dec 11, 2017 1:33 PM, "Michael Brohl" wrote:
I will see how I can handle this effectively without doing all my review
work
On a side note to Michael kudos on the massive bug hunting effort lately.
On Dec 11, 2017 1:33 PM, "Michael Brohl" 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
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
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
Thank you Michael!
Please see a minor comment inline:
On Mon, Dec 11, 2017 at 10:21 AM, Michael Brohl
wrote:
>
> I will check my latest commits and change this back where it is reasonable
> (i.e. where we have concatenations).
>
Actually this is not only relevant for
Yes better to keep it indeed
Jacques
Le 11/12/2017 à 10:05, Taher Alkhateeb a écrit :
Unless maybe something changed with the log4j API that I am unaware of. But
I suspect that's hard to implement
On Dec 11, 2017 12:01 PM, "Taher Alkhateeb"
wrote:
I agree with
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
Unless maybe something changed with the log4j API that I am unaware of. But
I suspect that's hard to implement
On Dec 11, 2017 12:01 PM, "Taher Alkhateeb"
wrote:
> I agree with Jacopo. String concatenation is a relatively expensive
> operation because strings are
I agree with Jacopo. String concatenation is a relatively expensive
operation because strings are immutable and this is exactly why the log4j
library provides the checking methods. I would also note that you need
these checking functions the most when the call is most utilized (verbose
or debug).
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
13 matches
Mail list logo