launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #25756
Re: [Merge] ~pappacena/launchpad:public-ecr-aws-ui into launchpad:master
Review: Needs Fixing
I think the field marked 'Region' should say 'Optional' or something. It's not clear (as a new user) that pushing to ROCKs or Dockerhub shouldn't need a 'region' field.
Also, I think it should appear in the overall push rules (picture 1), as you may have an image being pushed to multiple regions, with otherwise identical fields.
Diff comments:
> diff --git a/lib/lp/oci/browser/ocirecipe.py b/lib/lp/oci/browser/ocirecipe.py
> index 460b108..2103e1e 100644
> --- a/lib/lp/oci/browser/ocirecipe.py
> +++ b/lib/lp/oci/browser/ocirecipe.py
> @@ -380,6 +389,10 @@ class OCIRecipeEditPushRulesView(LaunchpadFormView):
> TextLine(
> __name__=self._getFieldName('url', elem.id),
> default='', required=True, readonly=True))
> + private_region_fields.append(
> + TextLine(
> + __name__=self._getFieldName('region', elem.id),
> + default='', required=True, readonly=True))
Is `required=True` correct here?
> private_username_fields.append(
> TextLine(
> __name__=self._getFieldName('username', elem.id),
--
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/394599
Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:public-ecr-aws.
References