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 defaults for optional args for 1.8.7 compatibility #27

Merged
merged 1 commit into from
Jun 13, 2011

Conversation

slothbear
Copy link
Contributor

Also remove ruby-debug19 for 1.8.7 compatibility.

This is my first pull request, so use the code, use pieces, ignore it, or whatever is appropriate. I wasn't sure of the correct style for the optional arguments. These changes made everything work ok in my project on 1.8.7. I didn't review all code, so there may be other 1.9.2 stuff still lurking.

Also, all specs pass on 1.8.7 except for one, which I couldn't figure out how to fix.

  1) Tanker Kaminari support should be able to return Kaminari compatible array for a search
     Failure/Error: array = Person.search_tank('hey!')
     NameError:
       uninitialized constant Tanker::KaminariPaginatedArray
     # ./spec/../lib/tanker.rb:153:in `paginate_results'
     # ./spec/../lib/tanker.rb:118:in `search'
     # ./spec/../lib/tanker.rb:251:in `search_tank'
     # ./spec/tanker_spec.rb:535

Also remove ruby-debug19, for 1.8.7 compatibility.
@arvida
Copy link

arvida commented Jun 12, 2011

Hi, I did most of these 1.8.7 incompatiblities. Sorry!
For the KaminariPaginatedArray initializer I think we could skip the optional argument for now as we are always giving the limit_val argument when using it, like:

class KaminariPaginatedArray < Array
...
def initialize(original_array, limit_val, offset_val, total_count)
  @limit_value, @offset_value, @total_count = limit_val, offset_val, total_count

I am looking in to the problem with the spec, there seem to be a problem with autoload and running code using KaminariPaginatedArray a second time in the spec on 1.8.7, if I remove the first kaminari support spec (should raise error..) it works fine..

@arvida
Copy link

arvida commented Jun 12, 2011

Update: #29

kidpollo added a commit that referenced this pull request Jun 13, 2011
Remove defaults for optional args for 1.8.7 compatibility
@kidpollo kidpollo merged commit feaf194 into kidpollo:master Jun 13, 2011
@kidpollo
Copy link
Owner

great work guys ! this is awesome ! Where are you guys located? I am want to send u guys Index Tank tshirts !

They are really awesome!

@arvida
Copy link

arvida commented Jun 13, 2011

awesome, thanks! i am in sweden, ill drop a mess with my address.

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