← Back to team overview

launchpad-reviewers team mailing list archive

Re: [Merge] lp:~cjwatson/launchpad/snap-change-code-check-privacy into lp:launchpad

 

Review: Approve code

I'd prefer seeing these all as Storm column validators rather than ugly prefixed properties.

Diff comments:

> 
> === modified file 'lib/lp/snappy/model/snap.py'
> --- lib/lp/snappy/model/snap.py	2019-02-26 06:47:45 +0000
> +++ lib/lp/snappy/model/snap.py	2019-03-29 17:17:13 +0000
> @@ -352,6 +354,29 @@
>          return ["snap:build:0.1"]
>  
>      @property
> +    def owner(self):
> +        return self._owner
> +
> +    @owner.setter
> +    def owner(self, value):
> +        # Public snaps may not be owned by private teams.
> +        if not self.private and value is not None and value.private:
> +            raise SnapPrivacyMismatch(
> +                "A public snap cannot have a private owner.")
> +        self._owner = value
> +
> +    @property
> +    def branch(self):
> +        return self._branch
> +
> +    @branch.setter
> +    def branch(self, value):
> +        if not self.private and value is not None and value.private:
> +            raise SnapPrivacyMismatch(
> +                "A public snap cannot have a private branch.")
> +        self._branch = value

Any reason not to do all these as Storm validators instead?

> +
> +    @property
>      def _api_git_path(self):
>          return self.git_path
>  


-- 
https://code.launchpad.net/~cjwatson/launchpad/snap-change-code-check-privacy/+merge/365294
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.


References