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

Make require: true the default for deleting models #1779

Merged
merged 2 commits into from Mar 4, 2018

Conversation

@ricardograca
Copy link
Member

@ricardograca ricardograca commented Mar 3, 2018

Introduction

Change Model.destroy() so that the require: true option is the default.

Motivation

This makes .destroy() behave the same way as save() and also matches what the documentation already says.

Current PR Issues

This is a breaking change so it must be clearly marked as such and there should be some kind of document to help users with the transition to this new behavior.

Alternatives considered

Not addressing this issue right now, but that would just be delaying the inevitable. It's better to do it in a 0.x release where some breakage is expected.

@ricardograca ricardograca added this to To Do in Version 0.13.0 via automation Mar 3, 2018
@ricardograca ricardograca moved this from To Do to In Progress in Version 0.13.0 Mar 3, 2018
@ricardograca ricardograca changed the title Rg require destroy Make require: destroy the default for deleting models Mar 3, 2018
@ricardograca ricardograca changed the title Make require: destroy the default for deleting models Make require: true the default for deleting models Mar 3, 2018
Copy link
Contributor

@kirrg001 kirrg001 left a comment

LGTM

@ricardograca ricardograca merged commit c3b8a21 into master Mar 4, 2018
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
Version 0.13.0 automation moved this from In Progress to Done Mar 4, 2018
@ricardograca ricardograca deleted the rg-require-destroy branch Mar 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants