Allow use with Rails 4 edge #820

Merged
merged 3 commits into from Mar 7, 2012

Conversation

Projects
None yet
2 participants
Collaborator

twalpole commented Mar 7, 2012

Tests updated to all pass when using rails edge. Biggest change was allowing [] stubs for errors object to match on either string or symbol since the tags in rails edge store the method name as strings now and index into the errors object using strings. The errors object calls #to_sym on any index though so @object.errors[:abc] is equivalent to @object.errors["abc"]

@justinfrench justinfrench and 1 other commented on an outdated diff Mar 7, 2012

@@ -9,3 +9,8 @@ end
appraise 'rails-3.2' do
gem 'rails', '~> 3.2.0'
end
+
+appraise 'rails-4' do
+ #gem 'rails', :git => 'git://github.com/rails/rails.git'
+ gem 'rails', :path=>'/Volumes/HOME/Users/twalpole/Projects/rails'
@justinfrench

justinfrench Mar 7, 2012

Owner

You'll need to fix this so that we're using git, not your local path :)

@twalpole

twalpole Mar 7, 2012

Collaborator

sorry about that -- had swapped over to a local repository to debug an issue, forgot to reset it -- pushed an update

@justinfrench justinfrench commented on an outdated diff Mar 7, 2012

gemfiles/rails-4.gemfile
@@ -0,0 +1,7 @@
+# This file was generated by Appraisal
+
+source :rubygems
+
+gem "rails", :path=>"/Volumes/HOME/Users/twalpole/Projects/rails"
@justinfrench

justinfrench Mar 7, 2012

Owner

Assume this was generated from appraisal, but again, local paths bad :)

@justinfrench justinfrench commented on the diff Mar 7, 2012

lib/formtastic/inputs/select_input.rb
@@ -195,7 +195,7 @@ def input_options
end
def input_html_options
- extra_input_html_options.merge(super)
+ extra_input_html_options.merge(super.reject {|k,v| k==:name && v.nil?} )
@justinfrench

justinfrench Mar 7, 2012

Owner

What was this about?

@twalpole

twalpole Mar 7, 2012

Collaborator

There is a test for select boxes that when :name=>nil is passed it still puts the correct name in the html output. In Rails 3.x this worked fine since it basically does options[:name] ||= so passing a name of nil gets ignored. In rails edge it uses options.fetch(:name) { } so the nil doesnt get ignored. To make the test pass this code removes the :name option from the html_options if its nil

and others added some commits Mar 3, 2012

@justinfrench @twalpole Merge pull request #818 from ZachBeta/master
Added a custom hash to the collections section of the readme
1303ccf
@twalpole twalpole swap to rails on github 7c538a7
Collaborator

twalpole commented Mar 7, 2012

@justinfrench hmm - not sure what happened with my update to have it reference rails on github instead of the local path, it seems to have merged in with pull request #818 or something. Let me know if its usable as is, or whether I need to fix something up.

Owner

justinfrench commented Mar 7, 2012

@twalpole usable as-is, pulling in now, thanks

justinfrench merged commit e4f3b04 into justinfrench:master Mar 7, 2012

@justinfrench justinfrench added a commit that referenced this pull request Mar 8, 2012

@justinfrench justinfrench Revert "Merge pull request #820 from twalpole/rails4"
This reverts commit e4f3b04, reversing
changes made to 0f42a5c.

Work still exists in rails4 branch until ready to re-merge.
363d51e
Owner

justinfrench commented Mar 8, 2012

@twalpole

Travis CI is failing under 1.8.7, REE and 1.9.2 (so only 1.9.3 is passing).

http://travis-ci.org/#!/justinfrench/formtastic/jobs/817527
http://travis-ci.org/#!/justinfrench/formtastic/jobs/817526
http://travis-ci.org/#!/justinfrench/formtastic/jobs/817525

I can certainly repeat the last two failures (BasicObject) under 1.8.7 on my local machine, haven't looked into the 1.9.2 issue.

I think the primary cause is that Rails 4 is dropping 1.8.7/REE, so the current build matrix where all 4 versions of Rails are running in all 4 ruby versions is flawed. Given this, I created a rails4 branch off master@e4f3b04 (with your stuff still in it) and reverted the changes in master.

We'll have to figure out a better build matrix with environment variables etc, so this work can be done in the rails4 branch and merged in when we're green and happy.

If you're interested in carrying the Rails 4 torch, I can add you as a contributor to get this branch rocking and keep an eye out for issues against Rails edge.

Collaborator

twalpole commented Mar 8, 2012

@justinfrench sounds good to me

Owner

justinfrench commented Mar 8, 2012

@twalpole done

@twalpole twalpole added a commit that referenced this pull request Mar 14, 2012

@twalpole twalpole Merge branch 'master' into rails4
* master:
  ignore .ruby-version
  Revert "Merge pull request #820 from twalpole/rails4"
ecf41d2

@twalpole twalpole added a commit that referenced this pull request Mar 14, 2012

@twalpole twalpole Revert "Revert "Merge pull request #820 from twalpole/rails4""
This reverts commit 363d51e.
f39d1bd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment