← Back to team overview

launchpad-reviewers team mailing list archive

Re: [Merge] ~pappacena/launchpad:ocirecipe-git-instructions into launchpad:master

 

Review: Needs Information



Diff comments:

> diff --git a/lib/lp/oci/browser/ocirecipe.py b/lib/lp/oci/browser/ocirecipe.py
> index f18779d..7019331 100644
> --- a/lib/lp/oci/browser/ocirecipe.py
> +++ b/lib/lp/oci/browser/ocirecipe.py
> @@ -773,10 +773,28 @@ class OCIRecipeAddView(LaunchpadFormView, EnableProcessorsMixin,
>              "architectures are restricted and may only be enabled or "
>              "disabled by administrators.")
>  
> +    def setUpGitRefWidget(self):
> +        """Setup GitRef widget indicating the user to use the default
> +        oci project's git repository, if possible.
> +        """
> +        path = self.context.getDefaultGitRepositoryPath()
> +        widget = self.widgets["git_ref"]
> +        widget.setUpSubWidgets()
> +        widget.repository_widget.setRenderedValue(path)
> +        default_repo = self.context.getDefaultGitRepository()
> +        if default_repo is None:
> +            msg = (
> +                "The git repository for this OCI project was not created yet."
> +                "<br/>Check the <a href='{oci_proj_url}'>OCI project's page"
> +                "</a> for instructions on how to create it.")

This sort of thing should use lp.services.webapp.escaping.structured, to guard against mistakes.  I'm also not completely sure about the English here - I think I would say "the OCI project page" rather than "the OCI project's page".

I'm also not quite sure the semantics are quite right here.  Shouldn't we be recommending a git path corresponding to the OCI recipe's owner?

> +            msg = msg.format(oci_proj_url=canonical_url(self.context))
> +            self.widget_errors["git_ref"] = msg
> +
>      def setUpWidgets(self):
>          """See `LaunchpadFormView`."""
>          super(OCIRecipeAddView, self).setUpWidgets()
>          self.widgets["processors"].widget_class = "processors"
> +        self.setUpGitRefWidget()
>  
>      @property
>      def cancel_url(self):


-- 
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/395253
Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:ociproject-git-instructions.


References