teamhephy / slugbuilder

MIT License
2 stars 11 forks source link

Delete deprecated buildpack #6

Open duanhongyi opened 5 years ago

duanhongyi commented 5 years ago

Some buildpack has been deprecated, and it is recommended to delete in a certain time point in the future without backwards compatibility.

see: https://github.com/heroku/heroku-buildpack-multi.git https://github.com/heroku/heroku-buildpack-grails.git https://github.com/heroku/heroku-buildpack-play.git

kingdonb commented 5 years ago

I'm still using heroku-buildpack-multi, and so are we in some recent documentation that we've put out (eg. https://github.com/teamhephy/example-rails5-reactjs) as it solves problems that we have not solved yet in Workflow.

https://devcenter.heroku.com/articles/using-multiple-buildpacks-for-an-app

Until we support buildpacks:add and buildpacks as a property of an app, buildpack-multi is here to stay. I don't know that we will ever do that, I am in favor of keeping more configuration inside of the git repository as long as it is not secret. We use Heroku buildpacks, but that does not mean we should do everything that Heroku does... in this case, I don't think I agree with their decision, and I don't know any good reason to leave behind .buildpacks and the multi-buildpack. (Perhaps someone can fork and adopt it in case any changes need to be made in it?)

As far as grails and play, I see they've been deprecated, and the instructions now point to guides for building gradle apps and scala. I am not a Java expert and don't know how equivalent those things are, but if the supported methods for deploying those frameworks have moved onto newer buildpacks and methods, then we probably should too. I haven't followed all of the links, but are there newer buildpacks that we can simply drop in as replacements?

@duanhongyi Would you be willing to look into this a little deeper and submit some proposed changes in a PR? Maybe we can get this on the roadmap for Hephy Workflow v2.21.

kingdonb commented 5 years ago

Oh, I just found #7 - thanks!

Cryptophobia commented 5 years ago

Thanks for the PR @duanhongyi. I added some comments to https://github.com/teamhephy/slugbuilder/pull/7. Also thank you for the due dilligence @kingdonb !

Agreed that we should definitely keep heroku-buildpack-multi. It serves a great purpose and allows us to do things that single buildpacks do not, without adding extra functionality directly in the slugbuilder component like @kingdonb mentioned.

The deprecation warning is also a soft warning for the multi buildpack:

There is no roadmap to taking `heroku-buildpack-multi` away from its users, or
for any feature changes that would break it. Our public stance (and, in fact,
our internal stance) is that work on checked-in definitions of buildpack lists
will not happen in this repo, and that this buildpack is "finished."

As for the grails and play buildpacks, we should keep them for backwards compatibility. In any case, users should probably be moving to the newer buildpacks at some point. We can deprecate them in the future if we really want or keep them for backwards compatibility. If however, those old buildpacks interfere with the scala buildpacks and the newer versioned buildpacks, the choice would definitely be to remove the old non-maintained ones. :+1: