Skip to content
This repository was archived by the owner on Jun 17, 2020. It is now read-only.

Envied guideline on defaults - #240

Merged
dgertych-monterail merged 1 commit into
masterfrom
feature/envied-defaults-guideline
Nov 20, 2015
Merged

Envied guideline on defaults#240
dgertych-monterail merged 1 commit into
masterfrom
feature/envied-defaults-guideline

Conversation

@kubakrzempek

Copy link
Copy Markdown
Contributor

Add guideline on how to use envied defaults.

Review on Reviewable

Add guideline on how to use envied defaults.
@kubakrzempek kubakrzempek changed the title Update rails.md Envied guideline on defaults Aug 10, 2015
@jandudulski

Copy link
Copy Markdown
Contributor

👍

1 similar comment
@jcieslar

Copy link
Copy Markdown
Contributor

👍

@teamon

teamon commented Aug 18, 2015

Copy link
Copy Markdown
Contributor

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

@dgertych-monterail

Copy link
Copy Markdown

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

@kubakrzempek

Copy link
Copy Markdown
Contributor Author

@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.

@mkarbowiak

Copy link
Copy Markdown

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.
eval/envied#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

@kubakrzempek

Copy link
Copy Markdown
Contributor Author

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

@mkarbowiak

Copy link
Copy Markdown

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

dgertych-monterail pushed a commit that referenced this pull request Nov 20, 2015
@dgertych-monterail
dgertych-monterail merged commit 13d3a8b into master Nov 20, 2015
@dgertych-monterail
dgertych-monterail deleted the feature/envied-defaults-guideline branch November 20, 2015 11:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants