launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

Diff comments:

> 
> === modified file 'lib/lp/snappy/vocabularies.py'
> --- lib/lp/snappy/vocabularies.py	2017-01-04 20:52:12 +0000
> +++ lib/lp/snappy/vocabularies.py	2019-02-05 13:05:32 +0000
> @@ -62,26 +74,35 @@
>      """A vocabulary for searching snappy/distro series combinations."""
>  
>      _table = SnappyDistroSeries
> -    _clauses = [
> -        SnappyDistroSeries.snappy_series_id == SnappySeries.id,
> -        SnappyDistroSeries.distro_series_id == DistroSeries.id,
> -        DistroSeries.distributionID == Distribution.id,
> +    _origin = [
> +        SnappyDistroSeries,
> +        LeftJoin(
> +            SnappySeries,
> +            SnappyDistroSeries.snappy_series_id == SnappySeries.id),

SnappyDistroSeries.snappy_series_id will always be set, won't it? Don't think this needs to be an outer join.

> +        LeftJoin(
> +            DistroSeries,
> +            SnappyDistroSeries.distro_series_id == DistroSeries.id),
> +        LeftJoin(Distribution, DistroSeries.distributionID == Distribution.id),
>          ]
> +    _clauses = []
>  
>      @property
>      def _entries(self):
> -        tables = [SnappyDistroSeries, SnappySeries, DistroSeries, Distribution]
> -        entries = IStore(self._table).using(*tables).find(
> +        entries = IStore(self._table).using(*self._origin).find(
>              self._table, *self._clauses)
>          return entries.order_by(
> -            Distribution.display_name, Desc(DistroSeries.date_created),
> +            NullsFirst(Distribution.display_name),
> +            Desc(DistroSeries.date_created),
>              Desc(SnappySeries.date_created))
>  
>      def toTerm(self, obj):
>          """See `IVocabulary`."""
> -        token = "%s/%s/%s" % (
> -            obj.distro_series.distribution.name, obj.distro_series.name,
> -            obj.snappy_series.name)
> +        if obj.distro_series is None:
> +            token = obj.snappy_series.name
> +        else:
> +            token = "%s/%s/%s" % (
> +                obj.distro_series.distribution.name, obj.distro_series.name,
> +                obj.snappy_series.name)
>          return SimpleTerm(obj, token, obj.title)
>  
>      def __contains__(self, value):


-- 
https://code.launchpad.net/~cjwatson/launchpad/snappy-series-can-guess-distro-series/+merge/362731
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snappy-series-can-guess-distro-series into lp:launchpad.