Code review comment for lp://staging/~lottanzb/lottanzb/download-properties

Revision history for this message
Severin H (severinh) wrote :

Gave it a try and it works without a hitch. Great work! It looks like you've also added some useful methods to the backend.hubs.general module.

Some random comments:

- GtkAction "show_properties_dialog" should be translatable (it's probably Glade's fault).
- Priority strings should also be translatable ("Force", etc.).
- Priority strings could be moved to lottanzb.constants, similar to DownloadStatus.to_pretty_string. We'll probably need them elsewhere too.
- gobject.TYPE_STRING can be replaced by a simple str.
- Many if blocks in the update method are nearly identical. It might be possible to use some fancy for loop there.
- What about displaying "unknown" if ETA is None (None won't be translated anyway)?
- TimeDelta objects (duration) have a property "short" that produces a (more or less) pretty string representation.
- "Downloaded:" field could be streamlined a bit when the download is complete. Currently, it's set to something like "1.19 GiB / 1.19 GiB (100%)". Call it "Size:" instead for completed downloads? Not sure about that.
- "Storage path:" field can be quite long and bloat the window. Not sure how to tackle that. When connected to a local instance of SABnzbd, it would be desirable to provide a button that directly opens the storage path.

« Back to merge proposal