-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Add Crystal::Environment module #5261
Conversation
69baec5
to
22200f7
Compare
# ``` | ||
module Environment | ||
ENV_KEY = "CRYSTAL_ENV" | ||
ENV_VALUES = %i(development test staging production) |
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.
What if I want other environment names? What if I don't want staging
, because I consider that staging == production?
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 values are used only to auto-generate query methods like Crystal.env.development?
.
In 99% of cases these should be enough, but if you don't need/want to use those, you can always do Crystal.env.name == "foo"
to check for non-standard env.
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 is not nice:
if Crystal.env.staging? || Crystal.env.name == "integration"
# do some stuff
end
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.
I agree, in terms of solutions we could add Crystal::Environment.setup(keys)
macro replacing/extending defaults, or use method_missing
(which was deemed a bad idea before).
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.
I like the idea for a setup macro. Though I think the default envs are going to be enough in most cases
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.
Also I think staging should be removed. Amber uses dev/test/prod and so does Elixir’s mix, Rails etc. Heroku even recommends against staging. So I think the main 3 should be there and people can add others if they want. Thoughts?
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.
why this needed? this is app related stuff, not crystal itself. better to create nice app generator template like |
bad, confusing naming. I suppose it should be part of config shard. |
Closed because #5220 was closed. |
This PR supplements stdlib with basic functionality around
ENV["CRYSTAL_ENV"]
variable.Closes #5220.
Code from this PR ATM lives in a shard, available at https://github.com/Sija/crystal-environment.