[rfc][icetea-web] Resource Setter/Getter Refactoring

[Andrew Azores]

On 05/20/2014 04:42 PM, Andrew Azores wrote:
> On 05/20/2014 04:35 PM, Lukasz Dracz wrote:
>>
>> ----- Original Message -----
>> From: "Andrew Azores" <aazores at redhat.com>
>> To: "Lukasz Dracz" <ldracz at redhat.com>, "Jiri Vanek" <jvanek at redhat.com>
>> Cc: distro-pkg-dev at openjdk.java.net
>> Sent: Tuesday, May 20, 2014 3:51:56 PM
>> Subject: Re: [rfc][icetea-web] Resource Setter/Getter Refactoring
>>
>> On 05/20/2014 02:35 PM, Lukasz Dracz wrote:
>>> ----- Original Message -----
>>> From: "Jiri Vanek" <jvanek at redhat.com>
>>> To: "Andrew Azores" <aazores at redhat.com>, "Lukasz Dracz" 
>>> <ldracz at redhat.com>
>>> Cc: distro-pkg-dev at openjdk.java.net
>>> Sent: Tuesday, May 20, 2014 11:20:56 AM
>>> Subject: Re: [rfc][icetea-web] Resource Setter/Getter Refactoring
>>>
>>>
>>>> Otherwise I think this is good to go. Please send a new version 
>>>> with fixed formatting and testSetSize.
>>>>
>>>> Thanks,
>>>>
>>>> And new changelog!
>>>>
>>>> Thanx for clean up! Good start!-)
>>>>
>>>> J.
>>> Hello,
>>>
>>> I changed the '10' to a constant in testIncrementTransferred(), 
>>> changed my eclipse format settings to use 4 spaces for indentation 
>>> and added the entry to the Changelog as suggested by Jiri.
>>>
>>> Thanks everyone for the help,
>>> Lukasz D
>>>
>>>
>>> Hi,
>>>
>>> +    public void setDownloadVersion(Version dVersion) {
>>> +        downloadVersion = dVersion;
>>> +    }
>>>
>>>
>>> This kind of thing was fixed for setTransferred, so I'd like to see it
>>> fixed here as well, and for setSize, and any others I may have missed.
>>>
>>> The comment on getTracker is also kind of questionable. I don't know if
>>> the @return is adding anything helpful here, and in any case, it's
>>> worded a bit awkwardly.
>>>
>>> Finally, where you have
>>> "resource.incrementTransferred(downloadLocationFile.length())", are you
>>> sure that this replacement is equivalent to the original line, which
>>> directly set the "transferred" field to this value rather than
>>> incrementing it? ie are you sure that at this point, "transferred" will
>>> always be 0? Perhaps a comment here stating this would be useful, so
>>> that it's clear that this is really a "set transferred" even though it
>>> appears to be an increment.
>>>
>>> Thanks,
>>>
>>> -- 
>>> Andrew A
>> Hello,
>>
>> Thank you for your suggestions I have applied them. As for the 
>> incrementTransferred I have changed it to setTransferred to ensure it 
>> works as previously defined for sure.
>>
>> Thanks,
>> Lukasz Dracz
>
> Looks okay to go to me.
>
> Thanks,
>

Actually one note, sorry - your in-patch ChangeLog isn't formatted 
properly, everything is indented too much. I will fix this myself before 
committing though.

Thanks,

-- 
Andrew A