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

Set memory default for Node builds #861

Closed
wants to merge 4 commits into from

Conversation

jmorrell
Copy link

@jmorrell jmorrell commented Mar 8, 2019

Fixes #859

My attempt at trying to address #859. I'm mostly guessing how to do this and happy to revise.

@jmorrell jmorrell requested a review from schneems March 8, 2019 22:35
Copy link
Contributor

@schneems schneems left a 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

#
# 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"
Copy link
Contributor

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.

@jmorrell jmorrell force-pushed the add-node-build-memory-default branch from 83f45b7 to bfaefb6 Compare May 20, 2019 22:57
@jmorrell jmorrell requested a review from schneems May 20, 2019 22:57
@schneems schneems force-pushed the add-node-build-memory-default branch from 15d0098 to a440cae Compare May 22, 2019 18:30
@schneems schneems force-pushed the add-node-build-memory-default branch from a440cae to b4f9c0f Compare May 22, 2019 18:31
@schneems schneems self-requested a review May 22, 2019 18:43
Copy link
Contributor

@schneems schneems left a comment

Choose a reason for hiding this comment

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

LGTM

@schneems schneems force-pushed the add-node-build-memory-default branch from b4f9c0f to a3c97e4 Compare May 22, 2019 19:06
@schneems schneems self-requested a review May 22, 2019 19:12
Copy link
Contributor

@schneems schneems left a comment

Choose a reason for hiding this comment

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

Still LGTM

@schneems schneems closed this May 24, 2019
@schneems schneems deleted the add-node-build-memory-default branch May 24, 2019 16:41
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.

Set memory default for Node builds
2 participants