Skip to content
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

Detail environment variables usage in README.md #1086

Merged
merged 7 commits into from
Feb 24, 2022

Conversation

kenden
Copy link
Contributor

@kenden kenden commented Feb 4, 2022

Why the change

The section Environment Variables in the README only provides information about passing environment variables to the recipes.
Finding the information about how to create just variables from the shell environment variables can be difficult.

What's the change

The "Environment Variables" section now adds 2 subsections:

  • How to pass environment variables to recipes
  • Loading just variables from the shell environments variables

Additionally, the subsection "How to pass environment variables to recipes"
now includes information about the export setting, using dotenv, and the dotenv-load setting.

@kenden kenden changed the title Detail environment variable usage in README.adoc Detail environment variables usage in README.adoc Feb 4, 2022
Copy link
Owner

@casey casey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, these are good additions. I added some comments regarding wording and formatting.

README.adoc Outdated
@@ -936,7 +936,7 @@ The executable is at: /bin/just

==== Dotenv Integration

`just` will load environment variables from a file named `.env`. This file can be located in the same directory as your justfile or in a parent directory. These variables are environment variables, not `just` variables, and so must be accessed using `$VARIABLE_NAME` in recipes and backticks.
`just` will load environment variables from a file named `.env` if the setting [dotenv-load](https://github.com/casey/just#dotenv-load) is present. This file can be located in the same directory as your justfile or in a parent directory. These variables are environment variables, not `just` variables, and so must be accessed using `$VARIABLE_NAME` in recipes and backticks.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
`just` will load environment variables from a file named `.env` if the setting [dotenv-load](https://github.com/casey/just#dotenv-load) is present. This file can be located in the same directory as your justfile or in a parent directory. These variables are environment variables, not `just` variables, and so must be accessed using `$VARIABLE_NAME` in recipes and backticks.
`just` will load environment variables from a file named `.env` if [dotenv-load](https://github.com/casey/just#dotenv-load) is set. This file can be located in the same directory as your justfile or in a parent directory. These variables are environment variables, not `just` variables, and so must be accessed using `$VARIABLE_NAME` in recipes and backticks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, updated

README.adoc Outdated

There are two ways to pass environment variables to recipes: exporting `just` variables, or using an `.env` file.

===== Exporting `just` variables
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
===== Exporting `just` variables
==== Exporting `just` Variables

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I updated

README.adoc Outdated
Comment on lines 1129 to 1132
==== How to pass environment variables to recipes

There are two ways to pass environment variables to recipes: exporting `just` variables, or using an `.env` file.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this section is probably unnecessary

Suggested change
==== How to pass environment variables to recipes
There are two ways to pass environment variables to recipes: exporting `just` variables, or using an `.env` file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed it

README.adoc Outdated
@@ -1158,6 +1164,17 @@ a $A $B=`echo $A`:
echo $A $B
```

Note: the setting link:README.adoc#export[export] causes all `just` variables to be exported to recipes as enviroment variables.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Note: the setting link:README.adoc#export[export] causes all `just` variables to be exported to recipes as enviroment variables.
When link:README.adoc#export[export] is set, all `just` variables are exported as environment variables.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I updated

README.adoc Outdated
Comment on lines 1169 to 1171
===== Using an `.env` file

`just` can load an .env file if the setting link:README.adoc#dotenv-load[dotenv-load] is present. The variables in the file will be available as environment variables to the recipes. See link:README.adoc#dotenv-integration[here] for more information.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
===== Using an `.env` file
`just` can load an .env file if the setting link:README.adoc#dotenv-load[dotenv-load] is present. The variables in the file will be available as environment variables to the recipes. See link:README.adoc#dotenv-integration[here] for more information.
==== Loading Environment Variables from a `.env` File
`just` will load environment variables from a `.env` file if link:README.adoc#dotenv-load[dotenv-load] is set. The variables in the file will be available as environment variables to the recipes. See link:README.adoc#dotenv-integration[dotenv-integration] for more information.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I updated

README.adoc Outdated
Comment on lines 1173 to 1176
==== Loading `just` variables from the shell environments variables

Environment variables from the shell can be loaded as variables in `just` with the functions `env_var()` and `en_var_or_default()`, described link:README.adoc#environment-variables[here].
They could then be exported to the recipes using link:README.adoc#exporting-just-variables[export].
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that second part is redundant, because if they're loaded using env_var, they will already be exported to recipes by default.

Suggested change
==== Loading `just` variables from the shell environments variables
Environment variables from the shell can be loaded as variables in `just` with the functions `env_var()` and `en_var_or_default()`, described link:README.adoc#environment-variables[here].
They could then be exported to the recipes using link:README.adoc#exporting-just-variables[export].
==== Setting `just` Variables from Environments Variables
Environment variables can propagated to `just` variables using the functions `env_var()` and `env_var_or_default()`. See link:README.adoc#environment-variables[environment-variables].

Copy link

@qnerden qnerden Feb 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh really? Then I would add that detail, and also in #environment variable functions sections.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

edited

@casey
Copy link
Owner

casey commented Feb 7, 2022

The readme was recently converted from asciidoctor to markdown, so there'll be some merge conflicts to resolve. Sorry for the hassle!

@kenden
Copy link
Contributor Author

kenden commented Feb 8, 2022

The readme was recently converted from asciidoctor to markdown, so there'll be some merge conflicts to resolve. Sorry for the hassle!

And I learned adoc formatting just for this :)
Thanks for the comments; I will wait for #1093 to be merged to update the PR.

@kenden kenden changed the title Detail environment variables usage in README.adoc Detail environment variables usage in README.md Feb 23, 2022
dotenv-load is just set. Not 'to true'

Signed-off-by: kenden <[email protected]>
Signed-off-by: kenden <[email protected]>
#### Setting `just` Variables from Environments Variables

Environment variables can be propagated to `just` variables using the functions `env_var()` and `env_var_or_default()`. These variables will be available as environment variables to the recipes.
See [environment-variables](#environment-variables).
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two sections called "Environment Variables". Maybe we could rename

Environment Variables

(after Setting Variables from the Command Line)
to something different, like "Getting and exporting environment variables") ?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, right, so linking doesn't work? How about "Getting and Setting Environment Variables" as the header for the section that starts with "Assignments prefixed with the export keyword…"?

Copy link
Contributor Author

@kenden kenden Feb 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Linking worked, but it is a bit harder to navigate in the table of content. I edited as you proposed

Copy link
Owner

@casey casey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for updating this! A couple of comments.

README.md Outdated
Comment on lines 900 to 908
- `env_var(key)` — Retrieves the environment variable with name `key`, aborting if it is not present. It will be available as environment variable to the recipes.

```make
HELLO := env_var('HELLO')

test:
echo "${HELLO}"
```

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this section if potentially confusing. If an environment variable is set, it will be available to recipes, but you don't have to call env_var for that to be the case.

Suggested change
- `env_var(key)` — Retrieves the environment variable with name `key`, aborting if it is not present. It will be available as environment variable to the recipes.
```make
HELLO := env_var('HELLO')
test:
echo "${HELLO}"
```
- `env_var(key)` — Retrieves the environment variable with name `key`, aborting if it is not present.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hue. Indeed. I was convinced that environment variables (from the shell) were /not/ available to recipes.
So
my_var := env_var('TEST')
is just the same as

my_var := `echo ${TEST}`

I edited the example and removed the confusing text

README.md Outdated
echo "${HELLO}"
```

- `env_var_or_default(key, default)` — Retrieves the environment variable with name `key`, returning `default` if it is not present. It will be available as environment variable to the recipes.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- `env_var_or_default(key, default)` — Retrieves the environment variable with name `key`, returning `default` if it is not present. It will be available as environment variable to the recipes.
- `env_var_or_default(key, default)` — Retrieves the environment variable with name `key`, returning `default` if it is not present.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

edited

also:
remove confusing parts

Signed-off-by: kenden <[email protected]>
README.md Outdated Show resolved Hide resolved
@@ -899,6 +899,16 @@ This is an x86_64 machine

- `env_var(key)` — Retrieves the environment variable with name `key`, aborting if it is not present.

```make
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code blocks with make syntax are extracted and parsed, to make sure that they're correct syntax, so the test is failing when it reaches /home/user. This should be wrapped in a separate code block of type sh.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Fixed.

kenden and others added 2 commits February 24, 2022 10:10
Code blocks with make syntax are extracted and parsed, to make sure that they're correct syntax, so the test is failing when it reaches /home/user. This should be wrapped in a separate code block of type sh.

Signed-off-by: kenden <[email protected]>
@casey casey merged commit 88922b8 into casey:master Feb 24, 2022
@casey
Copy link
Owner

casey commented Feb 24, 2022

Nice, merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants