launchpad-reviewers team mailing list archive

[Clinton Fung <mp+472757@xxxxxxxxxxxxxxxxxx>]

Review: Approve

Approve with a nitpick

Diff comments:

> diff --git a/vbuilder/bundle.yaml b/vbuilder/bundle.yaml
> index 1dbddf2..96b8bd9 100644
> --- a/vbuilder/bundle.yaml
> +++ b/vbuilder/bundle.yaml
> @@ -126,7 +126,7 @@
>  {%-   set lp_sshkey = "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIFrjt0yytzrK9fQuG+6VgE6QStUbDmunlN7+Lv5XhmoL stg-launchpad@launchpad-bastion-ps5" %}
>  {%-   set modifiers_bos01 = '{"arm64": "10.43.0.10", "ppc64el": "10.43.0.23", "s390x": "10.43.0.15"}' %}
>  {%-   set modifiers_bos02 = '{"arm64": "10.44.0.13", "ppc64el": "10.44.0.19", "s390x": "10.44.0.14"}' %}
> -{%-   set modifiers_bos03 = '{"amd64": "10.144.0.206", "arm64": "10.144.0.127", "riscv64": "10.144.0.114", "s390x": "10.144.0.47"}' %}
> +{%-   set modifiers_bos03 = '{"amd64": "10.144.0.206", "arm64": "10.144.0.127", "ppc64el": "10.144.0.110", riscv64": "10.144.0.114", "s390x": "10.144.0.47"}' %}

Nit: What is the proper order for the elements of this list? On the one hand, if you follow the same order as the line it replaces, it makes it much easier to see where the changes are. On the other hand, alphabetical or numerical ordering makes it easier to find the bits you want at a later time (i.e. post merge when looking to make the next change).

Yet another option would be that we find a comfortable way to split the list into multiple lines for readability.

I think I prefer keeping the list as close to the prior version of the list as possible, just with an addition at the end of the list (i.e. to enable easier diff at MP time).

>  {%-   set name_prefix = "launchpad-buildd-qastaging" %}
>  {%-   set openstack_tenant_name = "vbuilder_staging_project" %}
>  {%-   set openstack_tenant_name_bos03 = "launchpad-vbuilder-staging_project" %}


-- 
https://code.launchpad.net/~lgp171188/launchpad-mojo-specs/+git/private/+merge/472757
Your team Launchpad code reviewers is subscribed to branch ~launchpad/launchpad-mojo-specs/+git/private:vbuilder.