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

Revert #15502, and add a deprecation warning instead. #15878

Merged
merged 1 commit into from Jun 23, 2014

Conversation

sgrif
Copy link
Contributor

@sgrif sgrif commented Jun 23, 2014

This is public API, and simple_form depends on the nil return value.
We need to go through a deprecation cycle to return a null object. If
people want hash access, they can access the hash.

Closes #15877

This is public API, and `simple_form` depends on the `nil` return value.
We need to go through a deprecation cycle to return a null object. If
people want hash access, they can access the hash.
rafaelfranca added a commit that referenced this pull request Jun 23, 2014
Revert #15502, and add a deprecation warning instead.
@rafaelfranca rafaelfranca merged commit 679626e into rails:master Jun 23, 2014
@@ -104,11 +104,11 @@ def key_conversion_required?
end

def association_key_type
@klass.column_for_attribute(association_key_name).type
@klass.type_for_attribute(association_key_name.to_s).type
Copy link
Contributor

@egilburg egilburg Jun 23, 2014

Choose a reason for hiding this comment

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

could .to_s be normalized inside the type_for_attribute method?

Copy link
Contributor Author

@sgrif sgrif Jun 23, 2014

Choose a reason for hiding this comment

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

We've been pushing to consistently use strings internally, rather than allowing both everywhere.

@rafaelfranca
Copy link
Member

rafaelfranca commented Jun 23, 2014

Weird. Simple Form build should had fail on master 😢

@sgrif
Copy link
Contributor Author

sgrif commented Jun 23, 2014

I didn't actually confirm the report, but the fact that it came up at all made it clear someone somewhere is relying on this behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants