jmeter-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Peter Doornbosch <peter.doornbo...@gmail.com>
Subject Re: [GitHub] jmeter issue #358: Checkstyle: LineLength max 165, AnonInnerLength 45 and ot...
Date Thu, 21 Dec 2017 09:37:22 GMT
I wonder why you break up lines like these

         useBrowserCompatibleMultipartMode = new
JCheckBox(JMeterUtils.getResString("use_multipart_mode_browser")); //
$NON-NLS-1$

into

            useBrowserCompatibleMultipartMode =
                    new
JCheckBox(JMeterUtils.getResString("use_multipart_mode_browser")); //
$NON-NLS-1$

IMHO this does not improve readability, but makes it worse.

Moreover, the description says something about "max line length 165",
the line above is only 120 chars.

I admit max line length is also a matter of personal style, but i
couldn't find any docs about what JMeter prescribes in this matter.
Given no official rules, i would be careful with changes that won't be
seen as improvements by all of us (your PR does contain a lot of
changes that _are_ evident improvements).

My 2cts.

Peter

2017-12-21 8:33 GMT+01:00 ham1 <git@git.apache.org>:
> Github user ham1 commented on the issue:
>
>     https://github.com/apache/jmeter/pull/358
>
>     Sorry, yeah I see how this is difficult to review. I tend to change things as I see
them in the file I'm editing. I will revise and split it up into one type of change per commit,
is that ok? It might be more than 10 files but should be easier to review.
>
>
> ---

Mime
View raw message