Closed pavelskipenes closed 3 years ago
Thanks for the review @jonasbn. Didn't notice the lower case recommendation on the docs. Thanks for pointing out. I'll choose to use config-path
.
I should have tested before requesting a new review... It seems like config-path
is converted into INPUT_CONFIG-PATH
which is not interpreted as a variable in bash (the dash -
is not recognized as a part of the variable name). I'll have to experiment a bit more to see what I can do. Of course I could just roll back to not using dashes (or using an underscore instead maybe?).
It seems like dashes -
in variables is a valid yaml syntax however problem arrives when actions is trying to convert it to bash variables. According to examples in this documentation they are using underscores in their input parameters. I've also tested bash syntax locally as seen below.
There is only one thing that is really weird... In the documentation you mentioned there is a section for input_id's which says
The
<input_id>
must start with a letter or_
and contain only alphanumeric characters,-
, or_
.
That means config-path
should work just fine but they are not. I'm kinda suspecting that the intended behavior was to convert dashes to underscores but that is not documented there as far as I can see.
I'll just convert everything to underscores for now.
Edit: I've tested the changed locally.
Hi @pavelskipenes
You contribution has been included with release 0.9.0, which has just made it to:
Thanks again for your contribution
Take care and stay safe
I've tested this with my docker image and it seems to be working fine.