Bug 57084 - Closed socket in BeanShellClient
Summary: Closed socket in BeanShellClient
Status: RESOLVED FIXED
Alias: None
Product: JMeter
Classification: Unclassified
Component: Main (show other bugs)
Version: Nightly (Please specify date)
Hardware: All All
: P2 enhancement (vote)
Target Milestone: ---
Assignee: JMeter issues mailing list
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-10-13 20:50 UTC by Graham Russell
Modified: 2014-10-15 20:53 UTC (History)
0 users



Attachments
Code patch (15.56 KB, patch)
2014-10-13 20:50 UTC, Graham Russell
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Graham Russell 2014-10-13 20:50:10 UTC
Created attachment 32107 [details]
Code patch
Comment 1 Felix Schumacher 2014-10-14 09:12:52 UTC
Date: Tue Oct 14 09:09:29 2014
New Revision: 1631689

URL: http://svn.apache.org/r1631689
Log:
Bug 57084 - BeanShellClient: Close socket after usage.
Bugzilla Id: 57084

Modified:
    jmeter/trunk/src/core/org/apache/jmeter/util/BeanShellClient.java
Comment 2 Felix Schumacher 2014-10-14 09:13:40 UTC
I only committed the close operation on the socket.

I haven't committed the formatting changes, since I don't know the exact policy for such cosmetic changes (and my eclipse formatting would have produced slightly different changes).

Thanks for the contribution.
Comment 3 Sebb 2014-10-14 10:19:14 UTC
The formatting changes mostly look OK, though not essential.

However we don't apply such changes in the same commit as a code change, as it makes it more difficult to review the change, and makes reverts harder (should they be needed)/

Generally each commit should fix a single issue.
Formatting changes should not be combined with other changes.
Comment 4 Graham Russell 2014-10-14 21:33:57 UTC
Fair points - I will keep them separate in future - I didn't this time as I had already created the patch before I noticed the closing of the socket and I thought it such a small change as to not warrant splitting it out.

On the topic of formatting, while I agree formatting is not essential - I do think it is actually quite important when trying to further improve the readability of code for people both new and old to the project. Consistent formatting also, for me, gives a better impression of the code base and makes it easier to read.

Do we have (or could I raise a ticket about) an eclipse Java code formatter (and maybe one for other IDEs if people use them, or just format code using eclipse http://blogs.operationaldynamics.com/andrew/software/java-gnome/eclipse-code-format-from-command-line)?
Comment 5 Sebb 2014-10-15 20:52:23 UTC
Bugzilla is not ideal for discussions.

Please use the developer list for discussions on formatting.
Comment 6 Sebb 2014-10-15 20:53:40 UTC
Changed the title to agree with the action taken