Code review comment for lp:~mhr3/unity/respect-icon-size-hint

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

Code-wise, looks ok; and even though it's a pretty nasty hack, I don't see any other way around it.

But on a side note:

I was under the impression that using libunity private is strictly limited to UnityCore? In which case this class should be moved there.

struct IconLoaderTask

This should be a class, not a struct. And it's starting to get confusing with members vs locals. From the diff it doesn't look like this should compile because you changed the max_width to base_max_width, but are still using max_width in the function. Members should have trailing '_'. I realise it's not part of the bug, but it could definitely use an code refactor.

review: Approve

« Back to merge proposal