add support for builder :make -> buildErlangMk#43
add support for builder :make -> buildErlangMk#43adamcstephens wants to merge 2 commits intocode-supply:mainfrom
Conversation
|
Would you mind back-filling some tests? |
|
Yes, no problem. |
|
Are there other cases you'd like to test, or is this simple one enough? |
| {:hex, name, version, _hash, beam_builders, _sub_deps, _, sha256} = dep.opts[:lock] | ||
| fetcher = %FetchHex{pkg: name, version: version, sha256: sha256} | ||
|
|
||
| builder = |
There was a problem hiding this comment.
There's no test covering this change. I'd prefer to remove the implementation of this nil check, over having no test. Sorry for being fussy!
There was a problem hiding this comment.
I added this because previously the cond below was not matching and the error was very unclear when troubleshooting why deps_nix was crashing. So this is error handling for users and potential new builders being introduced, as no cond clause evaluated to a truthy value provides no indication why its failing and on which dep.
How do you propose to test a code path that calls System.halt?
|
I actually don't care enough about this change to continue. Hence why it took me so long to get back to it. This builder is not popular, and I only added this for a dependency I ended up not using. |
|
OK. Apologies for being a drag on this. Out of interest, what was the dependency? |
|
No, it's ok, I apologize as well if that came off poorly. I'm just being honest that I'm not going to spend the time. I think it was this one: https://hex.pm/packages/dbus |
2 participants
Also handle the error clearly if the builder isn't detected.