launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

Review: Approve code



Diff comments:

> === modified file 'lib/lp/registry/model/person.py'
> --- lib/lp/registry/model/person.py	2019-08-07 00:04:58 +0000
> +++ lib/lp/registry/model/person.py	2019-08-21 17:10:05 +0000
> @@ -4088,6 +4096,17 @@
>                  column = row[index]
>                  index += 1
>                  decorator(result, column)
> +            if need_icon:
> +                icon = row[index]
> +                index += 1
> +                cache.icon = icon
> +            if need_api:
> +                logo = row[index]
> +                index += 1
> +                cache.logo = logo
> +                mugshot = row[index]
> +                index += 1
> +                cache.mugshot = mugshot

Is this hunk necessary? They're FKs, so loading the objects into the Storm cache should be sufficient AFAICS.

>              return result
>          return DecoratedResultSet(raw_result,
>              pre_iter_hook=preload_for_people,


-- 
https://code.launchpad.net/~cjwatson/launchpad/person-api-cache-logo-mugshot/+merge/371597
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.