Support resources of arbitrary size in rebar3#2956
Conversation
| -spec get_resource_type(tuple(), [resource()]) -> {ok, resource()}. | ||
| get_resource_type(Source, Resources) when tuple_size(Source) > 0 -> | ||
| Type = element(1, Source), | ||
| get_resource(Type, Source, Resources); |
There was a problem hiding this comment.
Location and Source aren't the same. In some cases the Location is pulled out of the tuple.
There was a problem hiding this comment.
Thanks for the feedback! The reason I originally flattened everything is that a custom resource of size 2-4 can have whatever it likes in its tuple elements, so I thought that being picky about Source and Location is maybe too specific, especially if all this generates is an error message (which may not be the case).
I restored the existing clauses, and merely extended the size 6 case to cover all sizes > 4. The main motivation for the PR is to properly add support a custom resource I am using for some private code, which needs 5 args and previously raised a mere Failed to fetch and copy dep:.
There was a problem hiding this comment.
Might be worth still calling the Source variable be Location to keep the old meaning.
There was a problem hiding this comment.
I thought Location is supposed to be just an element of Source and the whole tuple I am matching is therefore a Source. Have I got this wrong?
There was a problem hiding this comment.
I'm actually unsure, I was just going from having replaced Location={Type, ...} and argued for keeping naming consistent, but if the prior naming was wrong then the new one is probably fine.
I'm unsure what the original naming meant and which part of the tuple it referred to, and I haven't recently done the context loading again.
There was a problem hiding this comment.
Understandable. Is there anything more I should do for this?
aronisstav
force-pushed
the
feature/allow-resources-of-any-size
branch
from
f8af7eb to
54ce6ce
Compare
|
It is unclear whether the failed tests are related to the changes or not, but I doubt they are. Not re-running in case it is relevant to check errors. |
|
The MacOS tests should be fixed by rebasing, brew image changes caused weirdness in our CI runs and that should be fixed in main. I'm unsure what happened with the Windows run, it seems generally successful. |
Removes arbitrary restriction to 2, 3, 4, or 6-element tuples for (possibly custom) resource specifications.
aronisstav
force-pushed
the
feature/allow-resources-of-any-size
branch
from
54ce6ce to
3225be4
Compare
|
@ferd Rebased. Checks are happy now 😄 |
3 participants
Removes arbitrary restriction to 2, 3, 4, or 6-element tuples for (possibly custom) resource specifications.