launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Diff comments:

> 
> === modified file 'lib/lp/snappy/browser/snap.py'
> --- lib/lp/snappy/browser/snap.py	2016-05-24 04:45:38 +0000
> +++ lib/lp/snappy/browser/snap.py	2016-05-24 05:16:28 +0000
> @@ -388,6 +433,10 @@
>                  render_radio_widget_part(widget, value, current_value)
>                  for value in (VCSType.BZR, VCSType.GIT)]
>  
> +    @property
> +    def has_snappy_distro_series(self):
> +        return not getUtility(ISnappyDistroSeriesSet).getAll().is_empty()

Mostly it makes it easy to roll this out without breakage before we create Snappy[Distro]Series on production.

> +
>      def validate_widgets(self, data, names=None):
>          """See `LaunchpadFormView`."""
>          if self.widgets.get('vcs') is not None:
> @@ -478,11 +568,18 @@
>  
>      @property
>      def initial_values(self):
> +        initial_values = {}
> +        if self.context.store_series is None:
> +            # XXX cjwatson 2016-04-26: Remove this case once all existing
> +            # Snaps have had a store_series backfilled.

I think it would be more generally convenient if we headed that way, yes.  It's certainly meaningful to have it set without store_upload being True; that allows you to build snaps that you only upload after some kind of manual verification pass (either entirely manually, or with some future work to ask LP to schedule an upload job that it wouldn't create automatically).

> +            sds_set = getUtility(ISnappyDistroSeriesSet)
> +            initial_values['store_distro_series'] = sds_set.getByDistroSeries(
> +                self.context.distro_series).first()
>          if self.context.git_ref is not None:
> -            vcs = VCSType.GIT
> +            initial_values['vcs'] = VCSType.GIT
>          else:
> -            vcs = VCSType.BZR
> -        return {'vcs': vcs}
> +            initial_values['vcs'] = VCSType.BZR
> +        return initial_values
>  
>      def validate(self, data):
>          super(SnapEditView, self).validate(data)
> 
> === modified file 'lib/lp/snappy/model/snap.py'
> --- lib/lp/snappy/model/snap.py	2016-05-17 12:47:34 +0000
> +++ lib/lp/snappy/model/snap.py	2016-05-24 05:16:28 +0000
> @@ -291,6 +292,18 @@
>                      or not self.require_virtualized))]
>  
>      @property
> +    def store_distro_series(self):
> +        if self.store_series is None:
> +            return None
> +        return getUtility(ISnappyDistroSeriesSet).getByBothSeries(
> +            self.store_series, self.distro_series)

Shouldn't do.  Deleting a SnappyDistroSeries should just mean that you can't create or edit snaps into a state where they use that combination of SnappySeries/DistroSeries.  This property will return None, but that shouldn't be a big deal.

SnappySeries, on the other hand, can't really be deleted: they're like DistroSeries, in that you have to change their status instead.

> +
> +    @store_distro_series.setter
> +    def store_distro_series(self, value):
> +        self.distro_series = value.distro_series
> +        self.store_series = value.snappy_series
> +
> +    @property
>      def can_upload_to_store(self):
>          return (
>              config.snappy.store_upload_url is not None and


-- 
https://code.launchpad.net/~cjwatson/launchpad/snap-store-add-edit-views/+merge/294524
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.