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

Add on_delete to ForeignKeys #41

Merged
merged 2 commits into from Jun 7, 2018
Merged

Add on_delete to ForeignKeys #41

merged 2 commits into from Jun 7, 2018

Conversation

ghost
Copy link

@ghost ghost commented May 18, 2018

Since Django 2.0 on_delete is required so I added it to the old default as CASCADE.

Since Django 2.0 on_delete is required so I added it to the old default as CASCADE.
@safwanrahman
Copy link
Owner

any migration needed?

@ghost
Copy link
Author

ghost commented May 18, 2018

Sorry I forgot to add it to git. I edited the initial script if that is okay.

@mayait
Copy link

mayait commented May 21, 2018

Any plan to deploy this update?

@safwanrahman
Copy link
Owner

safwanrahman commented Jun 3, 2018

@neonwarp Any new migration is needed or need to add it in existing migrations?
As existing migration will not run anytime in the production environment.
Do you know if its handeled in the database end or in the django end?
(Sorry for the late reply)

@ghost
Copy link
Author

ghost commented Jun 4, 2018

I updated the initial script so it adds the on_delete to the fields and I believe it is handled in Django. Before v2 it was default to CASCADE so technically it should be the same but v2 now requires the attribute when defining a FK.

Maybe it is better to keep it in a new migration script although I think one should just do it and the package should come with the initial migration script but this is for you to decide :)

@safwanrahman
Copy link
Owner

It seems like its needed to be in the initial migration and django handle it form their end, not the database end!
Thanks for your PR @neonwarp
r+

@safwanrahman safwanrahman merged commit 3e7caa5 into safwanrahman:master Jun 7, 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.

None yet

3 participants