Sorry, for the confusing email formatting. Of course, the way it is
displayed in the mail does not reflect the original source code
formatting, but i think you catch my point: i'm not very happy with
breaking up lines that are only 120 chars long......

Cheers
Peter


2017-12-21 10:37 GMT+01:00 Peter Doornbosch <peter.doornbo...@gmail.com>:
> 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 <g...@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.
>>
>>
>> ---

Reply via email to