search

gjaldon / heroku-buildpack-phoenix-static

A Heroku buildpack for building Phoenix's static assets
MIT License
229 stars 224 forks source link

Var in release #88

Closed rccursach closed 5 years ago

rccursach commented 5 years ago

passed phoenix_ex var from lib/common.sh#load_config() to bin/release through a small file generated in $BUILD_DIR.

Calling load_config() from inside release caused error, because of the messages echoed from load_config() were considered part of the YAML output generated by the release script.

also changed the logic of load_config(), bc phoenix 1.3.x apps have a package.json on ./assets/, that wasn't detected. and moved the sourcing of phoenix_static_buildpack.config at the end to override what was already set from the defaults if the file is present.

gjaldon commented 5 years ago

This is awesome @rccursach! Thanks a lot :) Let me know if you've tested with a deploy and if it's good to merge. Hope you don't mind the minor change I in bin/release.

Should add automated tests and CI for this buildpack so we no longer need to manually test in the future

rccursach commented 5 years ago

Yeah, makes sense removing that IF, the var is already there always.

Good to merge :). Tested with 1.3.x on heroku and dokku and works fine .

(Also looked in other Buildpacks for how testing is done. it's a lot of work, but the buildpacks from heroku have pretty consistent code base, independently of what framework/lang are targeted for. 👓)

gjaldon commented 5 years ago

Awesome! Thanks a lot for this contribution 💜