launchpad-reviewers team mailing list archive

[Ines Almeida <mp+475535@xxxxxxxxxxxxxxxxxx>]

Diff comments:

> diff --git a/lib/lp/buildmaster/model/buildfarmjobbehaviour.py b/lib/lp/buildmaster/model/buildfarmjobbehaviour.py
> index 4dd258a..934ddb6 100644
> --- a/lib/lp/buildmaster/model/buildfarmjobbehaviour.py
> +++ b/lib/lp/buildmaster/model/buildfarmjobbehaviour.py
> @@ -98,6 +98,15 @@ class BuildFarmJobBehaviourBase:
>  
>      def extraBuildArgs(self, logger=None) -> BuildArgs:
>          """The default behaviour is to send only common extra arguments."""
> +        launchpad_urls = {
> +            "launchpad.net": "production",
> +            "qastaging.launchpad.net": "qastaging",
> +            "staging.launchpad.net": "staging",
> +        }
> +        launchpad_server_url = config.vhost.mainsite.get(
> +            "hostname", "launchpad.test"

I wouldn't add a default for this one I think... It feels like we'd be saying that the hostname is `launchpad.test` when it might not be the case. In the `devel` case it feels more right: it's `devel` if it's not any of our known environments.

Also quick nit that you left a `develop` in the diff line 11 above

> +        )
> +        launchpad_instance = launchpad_urls.get(launchpad_server_url, "devel")
>          return {
>              "arch_tag": self.distro_arch_series.architecturetag,
>              "archive_private": self.archive.private,


-- 
https://code.launchpad.net/~pelpsi/launchpad/+git/launchpad/+merge/475535
Your team Launchpad code reviewers is requested to review the proposed merge of ~pelpsi/launchpad:propagate-launchpad-context-to-builders into launchpad:master.