launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

Review: Approve code



Diff comments:

> 
> === modified file 'lib/lp/snappy/validators/channels.py'
> --- lib/lp/snappy/validators/channels.py	2017-03-27 19:28:36 +0000
> +++ lib/lp/snappy/validators/channels.py	2018-05-07 10:29:56 +0000
> @@ -35,19 +62,26 @@
>      LaunchpadValidationError.
>      """
>      tracks = set()
> +    branches = set()
>      for name in channels:
>          try:
> -            track, risk = split_channel_name(name)
> +            track, risk, branch = split_channel_name(name)
>          except ValueError:
>              message = _(
>                  "Invalid channel name '${name}'. Channel names must be of the "
> -                "form 'track/risk' or 'risk'.",
> +                "form 'track/risk/branch', 'track/risk', 'risk/branch', or "
> +                "'risk'.",
>                  mapping={'name': html_escape(name)})
>              raise LaunchpadValidationError(structured(message))
>          tracks.add(track)
> +        branches.add(branch)
>  
>      if len(tracks) != 1:
>          message = _("Channels must belong to the same track.")
>          raise LaunchpadValidationError(structured(message))
>  
> +    if len(branches) != 1:
> +        message = _("Channels must belong to the same branch.")
> +        raise LaunchpadValidationError(structured(message))

Why? I guess it makes the UI easier, but if that's all it should be XXXed.

> +
>      return True


-- 
https://code.launchpad.net/~cjwatson/launchpad/snap-channels-branch/+merge/345171
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.