Expand env vars #38
Expand env vars #38
Conversation
README.md
Outdated
| %% ["c_src/nif.c", | ||
| %% "${REBAR_DEPS_DIR}/foo/bar.c"]}]}. | ||
| %% | ||
| %% This is a _very_ good way to be able to use you code both |
README.md
Outdated
| %% This is a _very_ good way to be able to use you code both | ||
| %% as a top level app and a dependency. | ||
| %% | ||
| %% CAVEAT! Not using {} is broken for the moment. |
There was a problem hiding this comment.
Does this imply a TODO for the future? If so, maybe add one?
|
My two comments are not a code review, just minor nits I noticed. But github marks them as a review, for some reason. |
|
@tsloughter, @ferd, maybe promote @cjkjellander to collaborator in the repo? |
|
Those were valid points. Fixed them. |
|
@Tuncer @ferd @tsloughter I've promoted @cjkjellander. |
|
@blt, thanks. Not sure why GitHub won't allow me to select @cjkjellander when requesting a review in #39. I was under the impression all collaborators would be listed. Must be a GitHub glitch. |
|
@cjkjellander, are you sure you've accepted the invitation to become a collaborator? I can't choose you from the reviewers list. |
|
@cjkjellander, merged the OTP 21 deprecation fix, since you approved the changes. |
|
@cjkjellander, any idea when the rebar3 changes will land? |
cjkjellander
force-pushed
the
expand_env_vars
branch
from
ec20262 to
f46e4ce
Compare
|
@Tuncer I had missed the collaboration email. About the deprecation fix, the function_exported/3 trick works if the module is loaded, but sting is stdlib so it should be sticky and loaded from the start. I tried the same trick in this branch I had to ensure my module was loaded first. I think the rebar3 changes are close to being merged. Rewrote them to fix the review comments. |
|
The fixes to rebar3 has landed in master. So this needs a review and a merge. |
|
Do you have a test case we could include before merging? |
Correct, it's one of many modules you can expect to be loaded, just like |
|
Since we have zero tests right now that might be a big project.... |
|
True, and to be precise, what I meant is if you have a project we can extract a test from. |
|
@asabil, @kape1395, if one of you wants to get write permissions in this repo, please say so and @blt will send an invite. The plugin needs active users who are willing to occasionally fix a bug. In fact, the sole reason pc isn't bundled with rebar3 since the removal during refactoring is that Fred and Tristan require more active/dedicated maintainer(s) for pc. Related: erlang/rebar3#1713 |
|
I will try to check the last pc version during this weekend. |
|
@cjkjellander, if you rebase the branch, I'll test it on my projects. |
cjkjellander
force-pushed
the
expand_env_vars
branch
from
9140ccf to
b56a837
Compare
|
@Tuncer it is rebased. I have two branches in two repos to test against. And then we have our code that didn't work with rebar3: https://github.com/campanja/csv/tree/rebar3-build Both of these should be built both as stand alone and as an included dep, and work in both ways of course. |
|
Thanks, it passes my local tests. |
ghost
merged commit
|
Tagged v1.7.0. @cjkjellander, can you publish to hex.pm? |
|
@Tuncer I'll try publishing. |
This fixes #30 and lets both env variables be exported and you can now use env variables from rebar3 inside port_specs, as they are expanded while evaluating the port_specs.
It depends on erlang/rebar3#1706 which in turn depends on erlang/rebar3#1698 but it is written so it shouldn't crash if using an older rebar3. Please check that it doesn't crash for you, before merge:ing.
Also I have updated the README to reflect how to use the env variables inside port_specs cause it depends on how rebar3 exports variables. Specifically, it needs ${} expansion, but at least it works.