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

[Lukasz Dracz]

----- 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 2014-05-20-P2-ResourceSetterGetter.patch
Type: text/x-patch
Size: 24023 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20140520/269c658f/2014-05-20-P2-ResourceSetterGetter-0001.patch>