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

Cleanup makefiles #2999

Merged
merged 6 commits into from
Aug 16, 2016
Merged

Cleanup makefiles #2999

merged 6 commits into from
Aug 16, 2016

Conversation

Kubuxu
Copy link
Member

@Kubuxu Kubuxu commented Jul 27, 2016

This can wait for after release but it shouldn't affect the release in any form either.

We might want to limit number of threads on the sharness or make output sequential of sharness because now they run in parallel.


deps: global-deps bins

clean:
rm $(BINS)
$(MAKE) -C sharness clean
-rm -rf $(BINS)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's -rm doing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

- prefix makes make ignore return code of rm command.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay then I think it's not needed -- -f makes it ignore non-existing files/dirs anyhow

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, I should clear it up everywhere.

@ghost
Copy link

ghost commented Jul 27, 2016

LGTM apart from one question ^

@atomgardner
Copy link
Contributor

You can squash these:

build install nofuse: deps
        $(MAKE) -C cmd/ipfs $@

clean uninstall:
        $(MAKE) -C cmd/ipfs $@

And if we could drop "deps" as a prerequisite here (it is serious overkill), all 5 options could be squashed into two lines.

@Kubuxu
Copy link
Member Author

Kubuxu commented Jul 28, 2016

@thomas-gardner that is awesome trick, Thanks,

but we can't drop the deps.
and the second one isn't really squashable.

@whyrusleeping
Copy link
Member

@Kubuxu wanna go ahead and rebase this on master?

License: MIT
Signed-off-by: Jakub Sztandera <[email protected]>
License: MIT
Signed-off-by: Jakub Sztandera <[email protected]>
License: MIT
Signed-off-by: Jakub Sztandera <[email protected]>
License: MIT
Signed-off-by: Jakub Sztandera <[email protected]>
License: MIT
Signed-off-by: Jakub Sztandera <[email protected]>
License: MIT
Signed-off-by: Jakub Sztandera <[email protected]>
@Kubuxu
Copy link
Member Author

Kubuxu commented Aug 4, 2016

done

@whyrusleeping whyrusleeping merged commit f2dcad8 into master Aug 16, 2016
@whyrusleeping whyrusleeping deleted the feature/makefile-cleanup branch August 16, 2016 17:15
@ghost ghost mentioned this pull request Dec 23, 2016
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