Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

extract retarget_path/2', relative_path/2' and `reduce_path/1' and add tests #800

Merged
merged 2 commits into from
Sep 25, 2015

Conversation

talentdeficit
Copy link
Contributor

useful in more contexts than just rebar_prv_common_test and this allows tests

they're currently duplicated in rebar_prv_common_test but will be removed in a separate pr once (if) this one is accepted

@ferd
Copy link
Collaborator

ferd commented Sep 13, 2015

This seems reasonable. I tend to associate reduce with a fold-like operation given map-reduce and python functionalities, but that's the only confusing thing there I believe.

@tsloughter
Copy link
Collaborator

What about rebar_dir:make_relative_path/2? Isn't that the same as this new rebar_file_utils function?

@talentdeficit
Copy link
Contributor Author

no, make_relative_path is bidirectional and will produce paths like ../../path and also doesn't do any kind of normalization of input. relative_path will only produce paths if they are ancestors of the base path.

the names are not great, they were taken directly from rebar_prv_common_test where they were private functions. the following name changes might make sense:

relative_path -> path_from_ancestor
reduce_path -> canonical_path

@ferd
Copy link
Collaborator

ferd commented Sep 14, 2015

Those seem fair. Would you expect canonical path to yield me an absolute path too?

@talentdeficit
Copy link
Contributor Author

it doesn't have to be an absolute path, just one that we can rely on to be stable on multiple calls to canonical_path so we can compare paths more easily. filename:absname is just an easy way to get that

@ferd
Copy link
Collaborator

ferd commented Sep 14, 2015

Sounds good then

`reduce_path/1` -> `canonical_path/1`
`relative_path/2` -> `path_from_ancestor/2`
ferd added a commit that referenced this pull request Sep 25, 2015
extract `retarget_path/2', `relative_path/2' and `reduce_path/1' and add tests
@ferd ferd merged commit 509c7af into erlang:master Sep 25, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants