add ability to dynamically make use of make -- minor #77

Closed
wants to merge 1 commit into from

4 participants

@expelledboy

No description provided.

@gustehn

Patch has passed first testings and has been assigned to be reviewed

@gustehn

Ok, I've got some feedback from the review for you:
The functionality is ok, but I think that the implementation should rather be an option to make:all/1 and make:files/2 instead of a new function i.e make:all([{emake,Emake}]). Where Emake is the same as the Emake arg to with/[1,2] in the patch. If present, Emake should be used instead of reading Emakefile from disk.
What do you think about that?

Also, it needs documentation and test, of course!

And I would like a better commit message which describes more exactly what the new functionality is.

@expelledboy

Okay thats easy enough to change code wise. I will have to look into how the docs are generated, and how your testcases are laid out. We dont really do either very well at the company I am working at :) so I have never had exposure to the otp way of doing things.

Might not be able to get to this this weekend.

@nox

@expelledboy The documentation is in lib/tools/doc/src/make.xml and tests are in lib/tools/test/make_SUITE.erl.

@garazdawi

closed due to inactivity, please re-open if you ever get around to completing this pr.

@garazdawi garazdawi closed this Aug 14, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment