Closed kubakrzempek closed 8 years ago
:+1:
:+1:
Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.
rails.md, line 411 [r1] (raw file): Why not just
enable_defaults! { ENV.fetch("RACK_ENV", "development").match(/development|test/) }
?
Comments from the review on Reviewable.io
rails.md, line 411 [r1] (raw file): This configuration doesn't allow you to setup global default through all stages Take this example under consideration: https://github.com/solarflarecommunications/solarcapture-web-ui/blob/master/Envfile
Most of the variables have same default through most of the stages. You don't want to set bunch of variables for each new stage.
We take different approach here and redefine only the variables which must be defined for current stage, and use once defined defaults.
Comments from the review on Reviewable.io
@teamon lambda or proc, it really doesn't matter.
@chytreg This configuration does what I believe ENVied was designed for - fail-fast when a variable is missing and provide a default for bootstrapping in dev/test environments. If your configuration requires different setup - that's fine. It's just a guideline. However, probably a different approach could be used for your particular scenario, e.g. using ENVied to make sure all variables are set with conjuction with .env to set the variables shared across environments.
Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.
rails.md, line 411 [r1] (raw file): Are guidelines good place to call attention to some obstacles developer might fall into? Because when I was creating Envfile for Codetunes 2.0 I found out that specyfing groups is not similar to, for example, Gemfile. https://github.com/eval/envied/pull/16 this is still not merged and actually referenced in one of your comments. It could be nice to mention it.
Comments from the review on Reviewable.io
Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.
rails.md, line 411 [r1] (raw file): @teamon lambda or block, it really doesn't matter.
@chytreg This configuration does what I believe ENVied was designed for - fail-fast when a variable is missing and provide a default for bootstrapping in dev/test environments. If your configuration requires different setup - that's fine. It's just a guideline. However, probably a different approach could be used for your particular scenario, e.g. using ENVied to make sure all variables are set with conjuction with .env to set the variables shared across environments.
@mkarbowiak No, I don't think so, because what is an obstacle today might not be tomorrow. I don't wont to guide someone to use explicitly:
%i{ staging preprod production }.each do |stage|
group stage do
# ...
end
end
It's up to the developer in question to find the easiest and most effective solution at any time. Otherwise we would have to update guidelines with each release of ruby, rails, coffeescript, envied, and so on.
Comments from the review on Reviewable.io
Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion.
rails.md, line 411 [r1] (raw file): Is this going to be merged? cc @kubakrzempek @teamon @chytreg
Comments from the review on Reviewable.io
Add guideline on how to use envied defaults.