[Merge] lp:~mandel/udm/add-tests into lp:udm
Sergio Schvezov
sergio.schvezov at canonical.com
Tue May 13 18:22:50 UTC 2014
Review: Needs Fixing
41 +type watch interface {
for quick reading can you call this watchInterface or something of the sort? Same for proxy perhaps
43 + Chanel() chan *dbus.Message
Channel?
+ return 0, fmt.Errorf("DBus Error: %", reply.ErrorName)
errors should always be lower case to be able to concatenate better in logs et.al. DBus is fine since it's a proper name, but 'Error' should be 'error'
--
func (down *FileDownload) Throttle() (throttle uint64, err error) {
reply, err := down.proxy.Call(DOWNLOAD_INTERFACE, "throttle")
if err != nil {
return 0, err
}
if reply.Type == dbus.TypeError {
return 0, fmt.Errorf("DBus Error: %", reply.ErrorName)
}
if err = readArgs(reply, &throttle); err != nil {
return 0, err
}
return throttle, nil
}
How about
func (...) Blah() (...) {
return down.getUint64Value("throttle")
}
func (...) getUint64Value(valueName string) (uint64, error) {
reply, err := down.proxy.Call(DOWNLOAD_INTERFACE, valueName)
if err != nil {
return 0, err
}
if reply.Type == dbus.TypeError {
return 0, fmt.Errorf("DBus Error: %", reply.ErrorName)
}
var value uint64
if err = readArgs(reply, &value); err != nil {
return 0, err
}
return value, nil
}
rinse and repeat for progress, totalSize, et.al; similar logic for others
595 + . "gopkg.in/check.v1"
As mentioned in the previous Needs Fixing, you need to use launchpad.net/gocheck
Finally, can you run goimports against the code? It formats internal and external imports nicely.
--
https://code.launchpad.net/~mandel/udm/add-tests/+merge/219200
Your team Ubuntu Phablet Team is subscribed to branch lp:udm.
More information about the Ubuntu-reviews
mailing list