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

update heroku deployment guide for webpack #2834

Conversation

happysalada
Copy link

@happysalada happysalada commented Apr 7, 2018

What

update the heroku deployment guide for webpack

Comments

  • by default the buildpack doesn't use node's latest LTS so it's running node 6.9, I thought it would be a nice improvement to use the LTS version (even if that version changes)
  • It seems phoenix wants to use npm by default so I kept the compile file with npm.

refs #2827

@Gazler
Copy link
Member

Gazler commented Apr 7, 2018

I like the idea, but I don't think we should specify exact versions in the guide - we will have to update every time a new node version is released. Instead, we should simply link to the configuration section of the buildpack with a note.

Something like:

This phoenix static buildpack pack can be configured to change the node version and compile options. Please refer to this configuration section for details.

@happysalada
Copy link
Author

@Gazler updated the guide accordingly.
I didn't squash my commits to make the PR more readable. Let me know if you need me to.

@happysalada happysalada force-pushed the raphael/update_heroku_deployment_guide branch from 2bf69d4 to b321b48 Compare May 7, 2018 16:36
@happysalada
Copy link
Author

@Gazler just rebased off latest master. Let me know if you want any other changes.

@Gazler Gazler merged commit a2e2548 into phoenixframework:master May 8, 2018
@Gazler
Copy link
Member

Gazler commented May 8, 2018

Thanks <3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants