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 sanitize_conditions and use sanitize_sql instead of sanitize_conditions #25999

Merged
merged 1 commit into from Aug 19, 2016

Conversation

kamipo
Copy link
Member

@kamipo kamipo commented Jul 31, 2016

Because sanitize_conditions protected method is only used in one place.

@rails-bot
Copy link

rails-bot commented Jul 31, 2016

r? @schneems

(@rails-bot has picked a reviewer for you, use r? to override)

@@ -36,8 +36,7 @@ def sanitize_sql_for_conditions(condition)
else condition
end
end
alias_method :sanitize_sql, :sanitize_sql_for_conditions
alias_method :sanitize_conditions, :sanitize_sql
Copy link
Member

@rafaelfranca rafaelfranca Aug 17, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While it is only used internally in one place it is a public API method. Could you add a deprecation message and a CHANGELOG entry?

Copy link
Member Author

@kamipo kamipo Aug 17, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a protected method. Should we add a CHANGELOG entry?

Copy link
Member

@rafaelfranca rafaelfranca Aug 17, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

@kamipo kamipo Aug 17, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh... I see. I'll add a CHANGELOG entry later.

Because `sanitize_conditions` protected method is only used in one place.
@kamipo kamipo force-pushed the remove_sanitize_conditions branch from 64c7758 to 6eaf5c9 Compare Aug 17, 2016
@kamipo
Copy link
Member Author

kamipo commented Aug 17, 2016

I added a deprecation message and a CHANGELOG entry.

@rafaelfranca rafaelfranca merged commit c2af081 into rails:master Aug 19, 2016
2 checks passed
@kamipo kamipo deleted the remove_sanitize_conditions branch Aug 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants