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

Read environment variables #20

Merged
merged 2 commits into from
Oct 24, 2024
Merged

Conversation

mooreryan
Copy link
Contributor

Addresses #15.

  • I based the format on the working with files test.
  • I used the category of "operating system" as that is what the Rust cookbook uses for reading environment variables.

Copy link
Member

@lpil lpil left a comment

Choose a reason for hiding this comment

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

Thank you! I've left some notes

README.md Outdated Show resolved Hide resolved
// First, unset the PORT variable.
//
// We do this fist to get the environment into a known state. If the PORT
// variable is not already set, this will have no effect.
Copy link
Member

Choose a reason for hiding this comment

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

The comments should describe how perform each task, it shouldn't be telling a story. See the other examples for the style

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hopefully it now better matches what you want. It's more similar to the work_with_files.gleam example now.

Copy link
Member

@lpil lpil left a comment

Choose a reason for hiding this comment

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

Beautiful! Thank you for waiting too, CodeBEAM conf took over my life for a few weeks

@lpil
Copy link
Member

lpil commented Oct 21, 2024

Oops, the tests are failing!

@mooreryan
Copy link
Contributor Author

Oops, the tests are failing!

Looks like the javascript and erlang behavior on key not found is different. See here: lpil/envoy#3

Copy link
Member

@lpil lpil left a comment

Choose a reason for hiding this comment

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

Ahhh good find! Thank you so much!!

@lpil lpil merged commit da9ef26 into gleam-lang:main Oct 24, 2024
1 check passed
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.

2 participants