Code review comment for lp:~nick-dedekind/unity/lp1062107.preview-pre-caching

Revision history for this message
Nick Dedekind (nick-dedekind) wrote :

> 530 + virtual ~PreviewPreCacher();
> 551 + virtual void OnPreCacheComplete();
>
> Could we make those pure virtual fucntions?
>
> Or if not could we just do:
> 530 + virtual ~PreviewPreCacher() {}
> 551 + virtual void OnPreCacheComplete() {}
>

Can't be pure virtual because we need to instantiate a PreviewPreCacher.
Don't see any benefit in making them inline.

>
> 347 + if (index < 0 || index >=(int)GetResultCount())
>
> Does this need to be casted to an int?
>

Yes. GetResultCount returns an unsigned int. Comes from the dee model.

>
> 758 + int top_social_info_max_width = MAX(details_width -
> style.GetAppIconAreaWidth() - style.GetSpaceBetweenIconAndDetails(), 0);
> 772 + int max_width = MAX(0, geo_cr.width - 2*(geo_cr.width*0.1));
> 773 + int max_height = MAX(0, (geo_cr.height - TAIL_HEIGHT) -
> 2*((geo_cr.height - TAIL_HEIGHT)*0.1));
> 786 + title_->SetMaximumWidth(MAX(0, GetGeometry().width - geo.height -
> style.GetMusicDurationWidth() - layout_spacing*2));
>
> Lets use std::max() here.

Will do.

« Back to merge proposal