wordpress-charmers team mailing list archive

[Benjamin Allot <benjamin.allot@xxxxxxxxxxxxx>]

Review: Approve

Some inline comments that should not block the merge but can be changed before top approving if you want to.

Diff comments:

> diff --git a/Dockerfile b/Dockerfile
> index 1fe4cbd..7603e5c 100644
> --- a/Dockerfile
> +++ b/Dockerfile
> @@ -9,6 +9,10 @@ LABEL org.label-schema.version=${DIST_RELEASE}-${VERSION}
>  # HTTPS_PROXY used when we RUN curl to download Wordpress itself
>  ARG BUILD_DATE
>  ARG HTTPS_PROXY
> +ARG VERSION
> +
> +# Used by Launchpad OCI Recipe to tag version
> +LABEL org.label-schema.version=${VERSION:-5.6}

According to https://github.com/opencontainers/image-spec/blob/master/annotations.md, I would probably add the missing labels that could be useful here:
* org.opencontainers.image:
 * url
 * source
 * version
 * etc...

and their org.label-schema equivalent.

>  
>  # Launchpad OCI image builds don't support dynamic arg parsing. Skip until
>  # https://bugs.launchpad.net/launchpad/+bug/1902010 is resolved.
> @@ -60,7 +64,7 @@ RUN a2enconf docker-php \
>      && a2enmod rewrite
>  
>  # Install the main Wordpress code, this will be our only site so /var/www/html is fine
> -RUN curl -o wordpress.tar.gz -fSL "https://wordpress.org/latest.tar.gz"; \
> +RUN curl -o wordpress.tar.gz -fSL "https://wordpress.org/wordpress-${VERSION}.tar.gz"; \

Not related to this change but I thing downloading the sha1 and checking the archive before uncompressing it might make sense here

>      && tar -xzf wordpress.tar.gz -C /usr/src/ \
>      && rm wordpress.tar.gz \
>      && chown -R www-data:www-data /usr/src/wordpress \


-- 
https://code.launchpad.net/~mthaddon/charm-k8s-wordpress/+git/charm-k8s-wordpress/+merge/395494
Your team Wordpress Charmers is subscribed to branch charm-k8s-wordpress:master.