-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Set memory default for Node builds #861
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good other than the issue I mentioned
lib/language_pack/ruby.rb
Outdated
# | ||
# This passes an argument to all Node processes during the build, so that they | ||
# can take advantage of all available memory on the build dynos. | ||
ENV["NODE_OPTIONS"] ||= "--max_old_space_size=2560" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a user set-able option? If so we need to use lowercase env()
function instead of ENV
.
83f45b7
to
bfaefb6
Compare
15d0098
to
a440cae
Compare
a440cae
to
b4f9c0f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
b4f9c0f
to
a3c97e4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still LGTM
Fixes #859
My attempt at trying to address #859. I'm mostly guessing how to do this and happy to revise.