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

Added logging of bindings & fixed bindings bug #75

Conversation

pivotal-david-osullivan
Copy link
Contributor

Fixes a bug where Bindings are silently created from hidden dirs. These were ignored when ConfigMap were being created but not for Bindings themselves.

This PR also logs bindings as pairs of 'name & keys' that are identified during the build, since it can be difficult to know whether bindings have been found as expected during the build process. It also prints a line to indicate if no bindings were identified, for the same reason.

build.go Outdated Show resolved Hide resolved
@dmikusa
Copy link
Contributor

dmikusa commented Sep 23, 2021

For what it's worth, the driving factor behind this is that people really struggle with bindings. In many cases, users don't get the binding format correct, but the buildpack doesn't output anything in either case so it's not clear if a buildpack sees or does not see them. This makes debugging difficult.

This change moves them front and center, so it's obvious when they're being read by a buildpack which makes debugging easier.

samj1912
samj1912 previously approved these changes Sep 23, 2021
@samj1912 samj1912 added semver:minor A change requiring a minor version bump type:bug A general bug type:enhancement A general enhancement and removed type:bug A general bug labels Sep 23, 2021
…gs from hidden files/dirs

Signed-off-by: David O'Sullivan <davidos@vmware.com>
build.go Show resolved Hide resolved
build.go Show resolved Hide resolved
build.go Show resolved Hide resolved
@samj1912 samj1912 dismissed their stale review September 24, 2021 12:19

Added comments

build.go Show resolved Hide resolved
@pivotal-david-osullivan
Copy link
Contributor Author

Closing in favour of #79 which only fixes the bug mentioned in this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver:minor A change requiring a minor version bump type:enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants