[rfc][icedtea-web] TinyHttpdImpl refactor

[Andrew Azores]

On 08/01/2013 03:25 PM, Adam Domurad wrote:
> On 08/01/2013 11:15 AM, Andrew Azores wrote:
>> On 07/30/2013 01:25 AM, Jiri Vanek wrote:
>>> On 07/26/2013 05:34 PM, Andrew Azores wrote:
>>>> Changelog:
>>>> * tests/test-extensions/net/sourceforge/jnlp/TinyHttpdImpl.java
>>>> (TinyHttpdImpl): BufferedReader and DataOutputStream become member
>>>> variables rather than local to run. More informative/mnemonic
>>>> variable names
>>>> (writeErrorToPipe): new private method.
>>>> (writeToPipe): new private method.
>>>> (run): refactored to use writeToPipe and writeErrorToPipe, and
>>>> ensuring that streams are closed when no longer in use. More
>>>> informative/mnemonic variable names
>>>>
>>>> The patch looks big (relative to the size of the file) but it's
>>>> really almost entirely variable renaming and method extractions. :)
>>> Does not, I'm dealyed due to packaging stuff ;( Sorry
>>> Practicaly no objections to the code:
>>> * The unittests for this are however a must - for new method,
>>> constructors and especially for run - to ensure they behave as they
>>> behaved before. Not that the backward compatibility is important in
>>> tests extensions, but because it worked pretty well.
>>> I would rather this patch to be pushed to head only together with
>>> unittests, but if you swear, you can push in two pushes.
>>>
>>>   Thanx for refactoring!
>>>     J.
>>
>> Round two.
>>
>> Changelog:
>> *
>> tests/test-extensions-tests/net/sourceforge/jnlp/ServerAccessTest.java:
>> moved TinyHttpdImpl tests into new class
>> *
>> tests/test-extensions-tests/net/sourceforge/jnlp/TinyHttpdImplTest.java:
>> new class for testing TinyHttpdImpl specifically
>> * tests/test-extensions/net/sourceforge/jnlp/TinyHttpdImpl.java:
>> refactored. "port" field removed as it was unused. Server now continues
>> to run after an HTTP 404.
>> * tests/test-extensions/net/sourceforge/jnlp/ServerLauncher.java:
>> removed "port" argument from TinyHttpdImpl constructor call
>>
>> I decided to undo some of my deeper changes to TinyHttpdImpl since they
>> were really done in the first place just to make the code look cleaner
>> (not good reasoning...), but I found better ways to clean it up without
>> making the reader/writer variables into fields.
>>
>> Andrew A
>
> I believe you forgot a patch
>
> Cheers,
> -Adam

D'oh, this happens to me too frequently. Thanks.

Andrew A
-------------- next part --------------
A non-text attachment was scrubbed...
Name: TinyHttpdImpl_refactor.patch
Type: text/x-patch
Size: 11602 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20130801/5f6ae223/TinyHttpdImpl_refactor.patch 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: TinyHttpdImpl_tests.patch
Type: text/x-patch
Size: 13451 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20130801/5f6ae223/TinyHttpdImpl_tests.patch