launchpad-reviewers team mailing list archive

[Ines Almeida <mp+489676@xxxxxxxxxxxxxxxxxx>]

Looks good, cool change!
Just a suggestion on the naming

Diff comments:

> diff --git a/Makefile b/Makefile
> index 1f350bd..ea88d75 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -170,5 +170,19 @@ publish-tarball: build-tarball
>  		$(SWIFT_CONTAINER_NAME) $(SWIFT_OBJECT_PATH) \
>  		$(TARBALL_BUILD_PATH) turnip=$(TARBALL_BUILD_LABEL)
>  
> +copy-certificates:
> +	mkdir -p /var/lib/haproxy
> +	cat turnip.crt turnip.key > /var/lib/haproxy/default.pem
> +
> +copy-haproxy-turnip-http-config:
> +	cat /etc/haproxy/haproxy.cfg haproxy-turnip-http.cfg > /tmp/haproxy.cfg
> +	mv /tmp/haproxy.cfg /etc/haproxy/
> +
> +reload-haproxy: copy-certificates copy-haproxy-turnip-http-config
> +	systemctl reload haproxy
> +
> +install: reload-haproxy

IMO `install` is too generic and it feels like it will install 'turnip' as a whole instead of just haproxy.

Maybe we can name this one `install-frontend` or something more specific?

Actually would be great to actually have a `make install` that install things instead of following the commands manually to create the venv, download a ppa, and bootstrap... (but that can be a separate proposal)

> +
>  .PHONY: build check clean dist run-api run-pack test
>  .PHONY: build-tarball publish-tarball
> +.PHONY: copy-certificates copy-haproxy-turnip-http-config install reload-haproxy


-- 
https://code.launchpad.net/~lgp171188/turnip/+git/turnip/+merge/489676
Your team Launchpad code reviewers is requested to review the proposed merge of ~lgp171188/turnip:setup-haproxy-for-cgit-dev-environment into turnip:master.