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. >> >> >> ---