Code review comment for lp:~ericsnowcurrently/fake-juju/makefile-tweaks

Revision history for this message
Free Ekanayaka (free.ekanayaka) wrote :

I find it a bit odd to have those "duplicate" targets with a different implementation depending on whether JUJU_VERSION is set or not. Personally, I'd find more natural to have a JUJU_VERSIONS environment variable instead, that should be set to a space or comma separated list of versions you want to build, that defaults to all versions. This will both make the Makefile more natural and allow you to possibly build not only one specific version but also more than one version. If you want to keep the JUJU_VERSION variable, you could add support for that by making JUJU_VERSIONS=$(JUJU_VERSION) if it's set.

All in all, I'm fine with the idea of the branch, but find the implementation a bit convoluted.

I'm keeping the N/I vote to see what you think of the above, but since it's all relatively minor, I'll be fine if you want to keep the implementation like it.s

« Back to merge proposal