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

[Andrew Azores]

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,

-- 
Andrew A