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 unused expand_range_bind_variables method #7875

Merged
merged 1 commit into from
Oct 8, 2012

Conversation

senny
Copy link
Member

@senny senny commented Oct 8, 2012

this method was not used, not documented and not tested.

While I was looking through the code to implement #6984 I found that this method seems to have lost it's purpose.

this method was not used, not documented and not tested.
@senny
Copy link
Member Author

senny commented Oct 8, 2012

@rafaelfranca @steveklabnik do you know of any usage for this method? I think we can drop it since it's not used anywhere.

@rafaelfranca
Copy link
Member

Right. The last usage of this method was removed at 77c23b2

rafaelfranca added a commit that referenced this pull request Oct 8, 2012
remove unused `expand_range_bind_variables` method
@rafaelfranca rafaelfranca merged commit a9bf3a4 into rails:master Oct 8, 2012
@senny
Copy link
Member Author

senny commented Oct 8, 2012

@rafaelfranca thanks for the immediate response :)

@gucki
Copy link

gucki commented Jan 7, 2013

@senny Did you also implement #6984? :)

@senny
Copy link
Member Author

senny commented Jan 7, 2013

@gucki I did not yet finish that... let me check my local branch. My vague memory is that there was something holding me back from finishing this feature.

@senny
Copy link
Member Author

senny commented Jan 7, 2013

@gucki I checked again and the issue is, that the complete Sanitization currently is on the ActiveRecord::Base level. execute however is one level deeper on the connection. At the time I did not think the necessary refactoring was justified by this feature.

@gucki
Copy link

gucki commented Jan 15, 2013

@senny Thanks for the info. Imo it might be a good idea to refactor this anyway, as it doesn't really belong to ActiveRecord::Base but to the connection, don't you think?

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.

None yet

3 participants