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

cmd/cdi: split out to a go module of its own. #103

Merged
merged 1 commit into from
Jan 24, 2023

Conversation

klihub
Copy link
Contributor

@klihub klihub commented Jan 10, 2023

Turn 'cdi' test binary into a go module of its own. This should prevent dependencies specific to that binary from leaking into any CDI downstream dependencies.

Turn 'cdi' test binary into a go module of its own. This
should prevent dependencies specific to that binary from
leaking into any CDI downstream dependencies.

Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
@klihub klihub requested review from bart0sh and elezar January 10, 2023 16:21
@@ -49,6 +49,10 @@ bin/%:
$(Q)echo "Building $@..."; \
$(GO_BUILD) -o $@ ./$(subst bin/,cmd/,$@)

bin/cdi:
Copy link
Contributor

Choose a reason for hiding this comment

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

As a matter of interest, why is this target duplicated?

Copy link
Contributor

Choose a reason for hiding this comment

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

OK. Discussed this offline. This is the actual build target wherease the other bin/cdi target just defines the dependencies.

Copy link
Contributor

@elezar elezar left a comment

Choose a reason for hiding this comment

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

Looks good @klihub.

Thanks.

Copy link
Contributor

@bart0sh bart0sh left a comment

Choose a reason for hiding this comment

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

/lgtm

@bart0sh bart0sh merged commit 95791ea into cncf-tags:main Jan 24, 2023
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