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

chore(*) parametrize makefiles and tools #942

Merged
merged 3 commits into from
Jul 31, 2020

Conversation

jakubdyszkiewicz
Copy link
Contributor

Summary

Parametrize makefiles and tools for more flexibility of running it.

Signed-off-by: Jakub Dyszkiewicz <jakub.dyszkiewicz@gmail.com>
Signed-off-by: Jakub Dyszkiewicz <jakub.dyszkiewicz@gmail.com>
Signed-off-by: Jakub Dyszkiewicz <jakub.dyszkiewicz@gmail.com>
@jakubdyszkiewicz jakubdyszkiewicz requested a review from a team as a code owner July 31, 2020 09:23
.PHONY: test
test: ${COVERAGE_PROFILE} test/api test/k8s test/kuma coverage ## Dev: Run tests for all modules
test: ${COVERAGE_PROFILE} ## Dev: Run tests for all modules
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe you can just do:

test: ${COVERAGE_PROFILE} ${TEST_TARGETS} coverage

without for-loop

Copy link
Contributor

Choose a reason for hiding this comment

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

Nope, that does not work. Ping me offline to explain, but essentially when there is invocation to the same target within a single make execution, it will not execute the target only once.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The trick is that you can't. Current implementation did not run test/k8s because test/module target was already run with test/api

@jakubdyszkiewicz jakubdyszkiewicz merged commit 0635929 into master Jul 31, 2020
@jakubdyszkiewicz jakubdyszkiewicz deleted the chore/parametrize-makefiles branch July 31, 2020 10:47
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.

None yet

3 participants