Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Fix #24: improve error messages during compilation #25

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
4 participants

My goal with the added echo statements is to make it easier for people with custom buildpacks to debug the detect script. Hopefully that also solves issue #24.

@uhoh-itsmaciek uhoh-itsmaciek commented on the diff Jan 24, 2014

bin/compile
# we'll get errors later if these are needed and don't exist
chmod -f +x $dir/bin/{detect,compile,release} || true
+ echo "running detect script" | indent
+ # Unset -e so we can see error messages in detect
@uhoh-itsmaciek

uhoh-itsmaciek Jan 24, 2014

Doesn't this prevent detect from killing compile if detect fails?

@wkschwartz

wkschwartz Jan 26, 2014

So I thought line 69 would do that but frankly I'm not fluent in Bash. I can buy fruits and vegetables at the market, but it's not my native tongue. Open to suggestion. I would like to preserve the ability -- somehow -- to see the output from detect for the case when it fails.

@BRMatt

BRMatt Oct 17, 2014

Just been bitten by this today, I'd agree that it's best to allow detect to fail and give the dev a helpful error message.

@wkschwartz

wkschwartz Oct 17, 2014

So what should the code be? Like I said above, Bash is not my strong suit.

I don't understand the implications of this additional branch of the conditional and I bring up another minor point inline, but I think overall this approach is reasonable. It'd be great to have a clear indication of the failing step.

Contributor

kennethreitz commented Jun 12, 2014

All the extra echos add an excessive amount of visual pollution, in my opinion. Conciseness is key when curating buildpack output.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment