Match on dep name in overrides #2287
Conversation
This patch aims to make rebar3 delete a dependency by first looking for its name (fixing `del` on hex dependencies) and if that isn't found remove it as always, keeping its current behaviour with the `del` override.
|
Thanks, could you add a test for this? |
|
I know the deps tests can be confusing but I think override tests might be easy enough to add a test for this that just checks the configuration in the state ends up right after running the overrides on a config. |
|
Pinging just in case @tsloughter :) |
| {deps, TopDeps}, | ||
| {overrides, [ | ||
| {del, some_dep, [ | ||
| {deps, [other_dep]} |
There was a problem hiding this comment.
What is this inner part? Are you sure you need this? Shouldn't it just be {del, some_dep}?
There was a problem hiding this comment.
This allows to only override one entry in the deps tuple as defined in some_dep. Seemed right to me
There was a problem hiding this comment.
Oh, I did the test with the inner part to, as Fred said, override one the other_dep from some_dep, since in the issue the example you gave with eredis_cluster and ex_doc is the same.
There was a problem hiding this comment.
Oh, I completely misread this. It is kinda confusing to read...
So it is saying "delete dep 'other_dep' from 'some_dep's list of deps"?
There was a problem hiding this comment.
Yep. A real example would be the one you mentioned in the issue:
The eredis_cluster hex package includes ex_doc which can't be built with rebar3 so it fails. The solution is to override all deps but really this should work too:
{overrides, [{del, eredis_cluster, [{deps, [ex_doc]}]}]}.If you prefer the override in the test to be a one-liner too I can change that :)
There was a problem hiding this comment.
Thanks, no, this is good.
This patch aims to make rebar3 delete a dependency by first looking for
its name (fixing
delon hex dependencies) and if that isn't foundremove it as always, keeping its current behaviour with the
deloverride.Closes #2126.