Closed G-Rath closed 1 year ago
@G-Rath since this Action is able to determine the version of Terminus being installed, it is a bit odd that a user is required to set which version of the build tools plugin to install. In addition, this new change will always install the build tools plugin, but not every user will need this, thought it will always add additional time to the runners.
it is a bit odd that a user is required to set which version of the build tools plugin to install
The main reason is complexity, which stems from two places - the first is that GHA doesn't have good support for boolean inputs so you end up having to do string comparisons and then having to decide if you do true | false
, yes | no
, 1 | 0
etc (+ combine that with YAMLs handling of booleans & schema validators, etc); the second is with having the action figure the version out, since this is meant to be a lightweight composite action and its not as simple as just passing a version in - we have to do different things based on if we're using Terminus v3 or v2.
So I felt that an enum like this gives us the lesser of two evils.
Is there a specific reason users cannot simply add this as a step in their workflows?
Because then they need to know the different steps required for install the plugin depending on what version of Terminus they're using; and there might be different steps for Terminus v4.
This way they just need to know the major version of Terminus they're targeting.
this new change will always install the build tools plugin, but not every user will need this, thought it will always add additional time to the runners. ... What is the intention of installing these build tools for everyone that wants to use this workflow?
I'm not sure why you think it always install the tools, since both install steps are conditional.
@G-Rath I missed the conditionals. 🙈 🤐 🤣
If I am not mistaken, the second step (Install Terminus) captures the version in TERMINUS_RELEASE
, even when it is a user supplied value. If you export the TERMINUS_RELEASE
to the $GITHUB_ENV
again, you will have the installed terminus
version in the environment.
From there you could simplify the code into one step, using a bash conditional, then change the build-tools-plugin
to a yes/no
, with a default of no
and a conditional on the step: if: ${{ inputs.build-tools-plugin == 'yes' }}
.
Side note, if you do make this change, I recommend updating the input name to install-build-tools-plugin
, to make it more clear of its intention.
From there you could simplify the code into one step, using a bash conditional, then change the build-tools-plugin to a yes/no, with a default of no and a conditional on the step: if: ${{ inputs.build-tools-plugin == 'yes' }}.
The first section of my previous comment explains why I decided not to do it like this.
This is tracked internally as FEAT-987.
Concern/doubt has been expressed about adding terminus v2 support to anything, and this PR includes explicit terminus v2 support. Terminus v2 is EOL on Feb 7 2023.
@namespacebrian thanks for the update - given the EOL date being the end of this month, it's not worth supporting v2 (which also makes this simpler anyway).
I'm going to close this given v2 is now EOL and this is very straightforward for users to do themselves (e.g. - run: terminus self:plugin:install terminus-build-tools-plugin
); personally I think it would still be nice for this action to support as a way of making downstream workflows a bit smaller but it's now a one-liner I think it would only make sense for this action to support if it took a list of plugins to install similar to setup-php
but since GitHub Actions does not have any native support for that I don't think it's worth supporting due to the complexity it would require.
This adds support for installing the
Terminus Build Tools plugin
as part of setting up Terminus.I have confirmed that this works by using it on one of our internal projects.