-
Notifications
You must be signed in to change notification settings - Fork 643
Add env attribute to set environment variables for recipes #2957
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
1002fef to
588b0a3
Compare
|
Also as a general note, I initially generated this PR with LLM coding tools and then manually audited+tweaked some things. I dunno if I'm personally pretty bullish on AI-assisted coding, I do it all the time myself, and I take the position that in general a human developer is responsible for the code they submit to a project, but the project shouldn't care exactly how they generated that code. |
588b0a3 to
00772f5
Compare
ccbd44b to
8525d41
Compare
|
@casey it'd be great if you could take a look at this, I think this is a pretty simple change that yields a pretty useful feature |
casey
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good. Some minor tweaks.
A couple bigger questions:
Should exported environment variables should be available to recipe parameters?
[env("foo", "bar")]
baz x=`echo ${foo}.txt`:
Also, should they be available to env functions?
[env("foo", "bar")]
baz:
echo {{ env("foo") }}.txt
I think the answer to both is probably yes.
As far as AI goes, I have no problem at all with LLM and AI generated code, as long as you the human coder "vouches" for the code to the same degree as code you wrote yourself, which requires reviewing the whole thing in depth before submitting it.
| matches!( | ||
| self, | ||
| Attribute::Arg { .. } | Attribute::Group(_) | Attribute::Metadata(_), | ||
| Attribute::Arg { .. } | Attribute::Env(_, _) | Attribute::Group(_) | Attribute::Metadata(_), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These should be alphabetically sorted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Am I missing something here, the Attribute members are in alphabetical order here
|
Actually, maybe they shouldn't be available to |
The env attribute accepts two arguments (env_var_name, value) and can be used
multiple times per recipe to set environment variables:
[env('API_KEY', 'secret')]
[env('LOG_LEVEL', 'debug')]
deploy:
./deploy.sh
Co-authored-by: Casey Rodarmor <casey@rodarmor.com>
1d861c9 to
f0a2a1e
Compare
This is actually potentially confusing behavior with |
The env attribute accepts two arguments (env_var, value) and can be used multiple times per recipe to set environment variables without using the existing
$parametersyntax:Addresses #2825