-
Notifications
You must be signed in to change notification settings - Fork 63
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
Pin the version of rails that is used #1377
Conversation
This fails due to:
|
I recommend that we drop Rails 4.2 from the test matrix. |
Gemfile
Outdated
@@ -7,7 +7,7 @@ gemspec path: File.expand_path('..', __FILE__) | |||
gem 'byebug' unless ENV['TRAVIS'] | |||
gem 'pry-byebug' unless ENV['CI'] | |||
|
|||
gem 'activemodel', ENV['RAILS_VERSION'] if ENV['RAILS_VERSION'] | |||
gem 'rails', ENV['RAILS_VERSION'] if ENV['RAILS_VERSION'] |
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.
ActiveFedora should not depend on all of Rails. We have a number of applications that are just ETL with ActiveFedora.
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.
That was your change. I think you had done that in the Gemfile here just for testing purposes since a really old version of rails was getting pulled in. I'll try rolling it back and see if it works with the pinning of bundler.
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.
Sweet! That worked!
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.
Ugh, sorry. Thanks for fixing it.
This looks great. Can someone do a "squash merge"? |
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.
Thanks for this!
y'all know the actual code you merged, according to the github diff, seems to have nothing to do with rails (contra PR title), but does deal with pinning bundler versions? |
@jrochkind I think that was discussed above, here: #1377 (comment) |
rather than the version of activemodel.
This prevents the tests from installing rails 0.9.5, and not finding the generator code we depend on.