[prev in list] [next in list] [prev in thread] [next in thread] 

List:       spacewalk-devel
Subject:    [Spacewalk-devel] commit c8317564eecd3839769971a39535ea3e189849ac
From:       jmatthew () redhat ! com (John Matthews)
Date:       2008-10-20 15:53:53
Message-ID: 48FCA991.2070803 () redhat ! com
[Download RAW message or body]

Jesus M. Rodriguez wrote:
> John,
> 
> http://git.fedorahosted.org/git/?p=spacewalk.git;a=commitdiff;h=c8317564eecd3839769971a39535ea3e189849ac
>  
> One nitpick with the hardware_device_by_id query. Right justify the
> SQL keywords for example, the T of SELECT
> should line up with the M in FROM and the final E in WHERE. The rest
> of the query looks ok.
> 
> Why are you doing the cast if the DataResult has a type?
> 
> +                DataResult<HardwareDeviceDto> dr =
> +                    SystemManager.getHardwareDeviceById(hwId);
> +                if ((dr != null) && (dr.size() > 0)) {
> +                    sr.setHw((HardwareDeviceDto)dr.get(0));
> 
Just my own ignorance, will fix.

> Also, if you're expecting only ONE HardwareDevice, then I'd change
> getHardwareDeviceById method
> to retun the HardwareDeviceDto directly instead.
> 
> sr.setHw(SystemManager.getHardwareDeviceById(hwId));
> 
Makes sense, will change.

> Everything else looks ok. Good job.
> 
> jesus
> 

Thanks for looking this over

-John


[prev in list] [next in list] [prev in thread] [next in thread]