On Sat, Apr 2, 2016 at 10:50 PM, Vladimir Sitnikov <
sitnikov.vladi...@gmail.com> wrote:

> What is the preferred way of commenting on the line-by-line basis?
> I'm not used to comment on patch files.
>
I'll make a PR next time

>
> How do you do that?
>
> >+ log.warn(getName()+":applying OUTOFMEMORY PROTECTION, SampleResults are
> not added anymore, use NON-GUI mode"
> >+                    + " or configure property
> 'view.results.tree.max_nodes'");
>
> Should it log only once? It looks like it would log for each new result.
>

In the first patch (the one that removes first Node or row) there is this
issue. Not in the second one as I test if warningLabel is visble


> > serialVersionUID
>
> Technically speaking, serialVersionUID bump is not required for
> TableVisualizer. Is it?
>

Doing so ensure that if used in Distributed test and 2 versions of JMeter
are used, a clear message showing this will be displayed.


>
> >+ private AtomicLong currentSizeApproximation = new AtomicLong(0l);


> What is that?
>
It's a IN PROGRESS work that was not removed from patch.
The idea was for me to compute an approximation of the size of all nodes so
that warning would be even more accurate


>
> > + /**
> >+     * When the limit is reached, JMeter will not {@link SampleResult}
> anymore
> >+     * Setting to 0 disables the protection
> >+     */
> >+    private boolean applyOOMProtection() {
>
> javadoc is weird. Especially, "setting to 0" part of ti.
>

Indeed, it is more for the property.

But the general question was which patch do you prefer ?
Once we decide I'll fix those issues.
Thanks for your notes.

>
> Vladimir
>



-- 
Cordialement.
Philippe Mouawad.

Reply via email to