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 nodejs from the final package #955

Merged
merged 1 commit into from
Oct 6, 2016
Merged

Conversation

stevendanna
Copy link
Contributor

Encouraged by rails-coffee and uglifier, nodejs used up 51MB of disk
space! In retaliation rm and rf teamed up at tribal council and voted
her off the island.

We've moved the two gems that require a javascript runtime into a new
group in the Gemfile. They will still be installed by default, but
rake and rails server won't require them by default when starting
up.

Signed-off-by: Steven Danna [email protected]

@stevendanna
Copy link
Contributor Author

@raskchanky or @smith or someone who has worked on oc_id. "Pretend" I know almost nothing about rails. Does this seem safe/reasonable to you?

Encouraged by rails-coffee and uglifier, nodejs used up 51MB of disk
space!  In retaliation rm and rf teamed up at tribal council and voted
her off the island.

We've moved the two gems that require a javascript runtime into a new
group in the Gemfile.  They will still be installed by default, but
`rake` and `rails server` won't require them by default when starting
up.

Signed-off-by: Steven Danna <[email protected]>
@raskchanky
Copy link
Contributor

@stevendanna AFAIK, the asset pipeline was the only reason for needing nodejs, so assuming the asset compilation stuff still works, then it's probably fine. I've never actually tried this myself, though, so I might be completely wrong. 😁

Copy link
Contributor

@smith smith left a comment

Choose a reason for hiding this comment

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

@stevendanna I think Josh is right. The node stuff is a build-time dependency in our case. I haven't tried this PR but what you did seems reasonable.

@stevendanna stevendanna merged commit f590895 into master Oct 6, 2016
@stevendanna stevendanna deleted the ssd/remove-nodejs branch October 6, 2016 10:33
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.

3 participants