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

Remove use of heroku-buildpack-multi #319

Merged
merged 1 commit into from
Mar 16, 2016
Merged

Conversation

esauter5
Copy link
Contributor

@esauter5 esauter5 commented Mar 8, 2016

I was trying to use heroku-buildpack-multi and was getting failures when trying to push to Heroku. This led me to realize that Heroku natively supports using multiple build packs now, so there is no need to use heroku-buildpack-multi or have to worry about a .buildpack file. I simplified the instructions and removed references to .buildpack in the code. Now, all you have to worry about is running this from the CLI:

heroku buildpacks:set heroku/ruby
heroku buildpacks:add --index 1 heroku/nodejs

Review on Reviewable

@esauter5
Copy link
Contributor Author

esauter5 commented Mar 8, 2016

I'm now noticing that the docs previously described that you could use heroku-buildpack-multi or set up multiple buildpacks via the toolbelt. I actually mistakenly merged these ideas together in a previous update on the docs. I still agree it's best to just remove heroku-buildpack-multi in general and have one simple way to set this up. Thoughts?

@justin808
Copy link
Member

@esauter5 This looks great! Can you find one other person in our slack room to confirm these instructions work? Thanks!

@stevesuh
Copy link

When you run those CLI commands does the .buildpack file get updated? I have an existing .buildpack file with dependenices to heroku-buildpack-apt and heroku-bundle-config.

@esauter5
Copy link
Contributor Author

AFAIK, the .buildpack file is only used by heroku-buildpack-multi. In this case, I'm suggesting removing that. Then you just run those two commands and the buildpacks are updated on heroku. You don't need to worry about .buildpack.

This brings an interesting point though. I guess I hadn't thought about the possibility that people still might need other buildpacks. May need to update the docs a little more to detail this option

@justin808
Copy link
Member

@esauter5 Keep me posted.

@esauter5
Copy link
Contributor Author

@justin808 Will do. Hope to get to this in the next day or so.

@esauter5 esauter5 force-pushed the master branch 2 times, most recently from 2b900a5 to 31a47ec Compare March 14, 2016 22:20
@esauter5
Copy link
Contributor Author

This should be good. I added something at the end of the heroku deployment section pointing to heroku-buildpack-multi in case people need a custom buildpack option.

For more information, see [Using Multiple Buildpacks for an App](https://devcenter.heroku.com/articles/using-multiple-buildpacks-for-an-app)

If for some reason you need custom buildpacks that are not officially supported by Heroku ([see this page](https://devcenter.heroku.com/articles/buildpacks)), we recommend checking out [heroku-buildpack-multi](https://github.com/ddollar/heroku-buildpack-multi).
Copy link
Member

Choose a reason for hiding this comment

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

@justin808
Copy link
Member

This looks good. One suggestion added.

@esauter5
Copy link
Contributor Author

The old docs recommended and linked to ddollar's version, but then used the heroku one in an example. The heroku version is just a fork of ddollar's and appears to be deprecated according to this comment.

I tried using heroku's previously and was not able to get it to work. I'm open to linking to whichever though.

@justin808
Copy link
Member

@esauter5 Great catch! Yes, just last fall, the strategy changed. And the ddollar one has been more recently updated.

@@ -10,8 +10,7 @@ class HerokuDeploymentGenerator < Rails::Generators::Base

def copy_heroku_deployment_files
base_path = "heroku_deployment"
%w(.buildpacks
Procfile
%w(Procfile
Copy link
Member

Choose a reason for hiding this comment

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

we need to delete the .buildpacks file also.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh good catch! That file was hiding from me because it was a dot file.

@justin808
Copy link
Member

Let's:

  1. rebase on top of master
  2. remove .buildpacks

Then I'll release the next version!

Great job @esauter5!

…ce heroku supports multiple buildpacks natively.
@esauter5
Copy link
Contributor Author

Thanks for the help @justin808! This should be good.

@justin808
Copy link
Member

💯

:lgtm_strong:


Reviewed 2 of 4 files at r1, 1 of 2 files at r2, 2 of 2 files at r3.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


Comments from the review on Reviewable.io

justin808 added a commit that referenced this pull request Mar 16, 2016
Remove use of heroku-buildpack-multi
@justin808 justin808 merged commit 3a78962 into shakacode:master Mar 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants