Code review comment for lp:~wallyworld/launchpad/sharing-view-batching4-957663

Revision history for this message
j.c.sackett (jcsackett) wrote :

Hey Ian--

This mostly looks good; I just have one question below.

> === modified file 'lib/lp/registry/browser/pillar.py'
> --- lib/lp/registry/browser/pillar.py 2012-03-21 14:30:16 +0000
> +++ lib/lp/registry/browser/pillar.py 2012-03-22 13:04:20 +0000
> @@ -303,15 +303,17 @@
> size=config.launchpad.default_batch_size,
> range_factory=StormRangeFactory(sharees))
>
> - def shareeData(self):
> - """Return an `ITableBatchNavigator` for sharees."""
> + @property
> + def sharees(self):
> + """An `IBatchNavigator` for sharees."""
> if self._batch_navigator is None:
> - unbatchedSharees = self.unbatchedShareeData()
> + unbatchedSharees = self.unbatched_sharees
> self._batch_navigator = self._getBatchNavigator(unbatchedSharees)
> return self._batch_navigator
>
> - def unbatchedShareeData(self):
> - """Return all the sharees for a pillar."""
> + @property
> + def unbatched_sharees(self):
> + """All the sharees for a pillar."""
> return self._getSharingService().getPillarSharees(self.context)
>
> def initialize(self):

I'm not sure I understand the logic of making these properties; don't we
usually avoid that for things that might be expensive or slow, like
unbatched_sharees?

review: Needs Information

« Back to merge proposal