Skip to content
This repository has been archived by the owner on Mar 11, 2020. It is now read-only.

Interpolate environment variables in imports #54

Merged
merged 3 commits into from
May 6, 2017
Merged

Interpolate environment variables in imports #54

merged 3 commits into from
May 6, 2017

Conversation

rossabaker
Copy link
Contributor

Fixes #53

@rossabaker
Copy link
Contributor Author

A couple discussion points before we clear this for release:

  • Changed types of public importsOf and resolveImports. I'm not sure that these really should be public in the first place. This is, as it stands, a breaking change.
  • Should we be able to interpolate mutable names into imports, like config values and system properties? Or should import interpolation be limited to immutable values, like sys.env? I reused the binding interpolator here, but it might be considered too permissive when we start to think about things like file watching.

@timperrett
Copy link
Contributor

@rossabaker im not wild on a major version breaking change for this functionality. Is there a way it can be done in a bin-compat manner? Perhaps we could (should?) just document the cases that dont work (not a huge fan of this, but i don't want to have to upgrade all the users right now)

@rossabaker
Copy link
Contributor Author

This should pass MiMa (I'll add that later to be sure). It deprecates methods that neither worked nor are likely to be used.

Supports only environment variables in import directives. This is all the documentation ever promised (now made more explicit), and sidesteps all issues of how we monitor mutable bindings.

@timperrett
Copy link
Contributor

@rossabaker im happy with this :-) thanks!

@rossabaker rossabaker changed the title DO NOT MERGE: Interpolate imports Interpolate environment variables in imports May 6, 2017
@rossabaker rossabaker merged commit 2ee0438 into Verizon:master May 6, 2017
@rossabaker rossabaker deleted the issue-53 branch May 6, 2017 04:28
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.

2 participants