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

Target build asset for the Buildpack when using the environment variable #6

Merged
merged 2 commits into from
Oct 19, 2021

Conversation

shahulsonhal
Copy link
Member

@shahulsonhal shahulsonhal commented Oct 18, 2021

We have a Buildpacke e2e test in the shipwright-io/build repo that sets the BP_GO_TARGETS environment variable.

Note: See here the test case that uses the introduced changes.

We have a Buildpacke e2e test in the https://github.com/shipwright-io/build repo that sets the BP_GO_TARGETS environment variable.
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 18, 2021
@shahulsonhal shahulsonhal changed the title WIP: Target build asset for the Buildpack when using the environment variable Target build asset for the Buildpack when using the environment variable Oct 18, 2021
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 18, 2021
Copy link
Member

@SaschaSchwarze0 SaschaSchwarze0 left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 18, 2021
@SaschaSchwarze0 SaschaSchwarze0 removed their assignment Oct 18, 2021
@SaschaSchwarze0
Copy link
Member

/assign@adambkaplan

@adambkaplan the approve (and maybe automatic merge) still does not work here.

@SaschaSchwarze0
Copy link
Member

/assign adambkaplan

Copy link
Member

@SaschaSchwarze0 SaschaSchwarze0 left a comment

Choose a reason for hiding this comment

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

Sry, just noticed your directory structure. I think this needs to change. Imo building with contextDir=source-build without BP_GO_TARGETS will work because there is a main.go inside source-build. But we want a build config that fails if the env var is no set. So, imo you need to move it to a new directory, like /source-build-with-package/main-package

@SaschaSchwarze0 SaschaSchwarze0 removed the lgtm Indicates that a PR is ready to be merged. label Oct 18, 2021
Copy link
Member

@SaschaSchwarze0 SaschaSchwarze0 left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 19, 2021
Copy link
Member

@adambkaplan adambkaplan left a comment

Choose a reason for hiding this comment

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

/approve

@adambkaplan
Copy link
Member

Green button merging - looks like this was not fully integrated with OpenShift's CI system (which provides all of the prow commands and tide integration).

@adambkaplan adambkaplan merged commit 7dd0050 into shipwright-io:main Oct 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants