Skip to content
This repository has been archived by the owner on Jul 24, 2020. It is now read-only.

Move seed script gems to development and test groups #1164

Closed
wants to merge 1 commit into from

Conversation

mnquintana
Copy link
Contributor

We shouldn't be requiring these gems in production. Related to #1163, #1158, #600

We shouldn't be requiring these gems in production
@orenyk
Copy link
Contributor

orenyk commented Feb 20, 2015

We actually can't do this because of Heroku :-, I moved them out in #275.

@orenyk orenyk closed this Feb 20, 2015
@mnquintana
Copy link
Contributor Author

@orenyk Ehhh, I think that's a workaround. Like these seriously shouldn't be loaded in production at all - it's a waste of memory (and Reservations is already a serious memory hog). Why do we need to run the seed script in a Heroku deploy? Wasn't that just for testing?

@orenyk
Copy link
Contributor

orenyk commented Feb 20, 2015

It wasn't just for testing, by default if you click the "Deploy to Heroku" button it runs the seed script to give you a fully seeded Reservations instance. Unless we want to generate a separate seed script for Heroku that doesn't use the progress bar or ffaker, we're going to have to leave them, and I'd rather stick with a single seed script since otherwise any change we make to our objects might need to be dealt with twice (although we could mitigate this by writing tests for both scripts). If you really want we can remove the progress bar regardless (it's entirely unnecessary), but I'm not sure how to avoid loading ffaker. Maybe require: false in the Gemfile?

EDIT Note that we'd have to manually require them in the seed script. nvm, we already do

@mnquintana
Copy link
Contributor Author

No I don't want to remove features / niceties from the seed script, but I think there's a better way to do this. Couldn't we create like a :heroku group or something in the Gemfile and configure Heroku to install things in the :heroku group too? So then it would be something like:

group :development, :test, :heroku do
  gem 'ffaker', '~> 1.32.1'
  gem 'ruby-progressbar', '~> 1.7.1'
end

This way we wouldn't have to install them in production and we'd still only have one seed script. What do you think?

@orenyk
Copy link
Contributor

orenyk commented Feb 22, 2015

Yea, that's not a bad idea. Does Bundler install all groups by default? I think we can definitely require: false both of these regardless, no reason to have them required in Rails. I'll open up an issue for this and we'll get it done in the next sprint.

@orenyk orenyk deleted the mnquintana-patch-1 branch March 30, 2015 16:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants