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 obsolete fields_for/2 function #287

Merged
merged 1 commit into from
Jul 20, 2018

Conversation

moxley
Copy link
Contributor

@moxley moxley commented Jun 20, 2018

Fixes #157

Since version 1.0.0, when users of ex_machina try to call the deprecated fields_for/2 function, it raises an exception. This doesn't seem to provide any benefit. If instead it's removed, calls to it will fail at compile time. But if it stays in, calls will fail at runtime, possibly in production.

This PR solves the Dialyzer issue and it provides a better solution for dealing with the obsolete function.

@sbrink
Copy link

sbrink commented Jun 30, 2018

+1

@tielur
Copy link

tielur commented Jul 18, 2018

Would this fix these types of dialyzer issues?

________________________________________________________________________________
test/support/factory.ex:11:no_return
Function subject_factory/0 has no local return.
________________________________________________________________________________
test/support/factory.ex:3:no_return
Function fields_for/1 has no local return.
________________________________________________________________________________
test/support/factory.ex:8:no_return
Function patient_factory/0 has no local return.
________________________________________________________________________________

@moxley
Copy link
Contributor Author

moxley commented Jul 18, 2018

@tielur: Possibly. Try pulling this branch into your mix.exs deps() to find out.

@germsvel
Copy link
Collaborator

I think this makes sense to remove, especially since the deprecation was introduced in 1.0.0 and we're at 2+. Thanks @moxley for providing some info in the PR description!

@germsvel germsvel merged commit b17ba1c into beam-community:master Jul 20, 2018
@moxley moxley deleted the remove-fields-for branch July 21, 2018 02:18
@moxley
Copy link
Contributor Author

moxley commented Jul 21, 2018

❤️

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.

4 participants