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

Update increment! documentation [ci skip] #25427

Merged
merged 1 commit into from Jan 8, 2017

Conversation

eugeneius
Copy link
Member

The increment! and decrement! methods were reimplemented in #11410 to make them safe to call from multiple connections concurrently. This changed their behaviour in a few ways.

Previously they used update_attribute, which calls the attribute setter method, runs callbacks, and touches the record. Now they behave more like update_column, writing the update to the database directly and bypassing all of those steps.

The change in behaviour was pointed out on the original pull request: #11410 (comment)

@rails-bot
Copy link

Thanks for the pull request, and welcome! The Rails team is excited to review your changes, and you should hear from @pixeltrix (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@eugeneius
Copy link
Member Author

This brings the documentation back in line with reality, but the new behaviour here is set to ship in Rails 5, even though it constitutes several breaking changes.

#21979 tries to restore the previous touching behaviour, but the other differences also have the potential to break existing apps.

At the very least, I think we need to add a feature toggle to new_framework_defaults.rb for 5.0.

What do you think @sgrif?

@maclover7
Copy link
Contributor

r? @sgrif

@rails-bot rails-bot assigned sgrif and unassigned pixeltrix Jun 18, 2016
The `increment!` and `decrement!` methods were recently reimplemented to
make them safe to call from multiple connections concurrently. This
changed their behaviour in a few ways.

Previously they used `update_attribute`, which calls the attribute
setter method, runs callbacks, and touches the record. Now they behave
more like `update_column`, writing the update to the database directly
and bypassing all of those steps.
@eugeneius eugeneius force-pushed the update_increment_documentation branch from cffded5 to 115e314 Compare August 15, 2016 00:15
@eugeneius
Copy link
Member Author

5.0.0 shipped with this behavour - maybe at this point we should just update the documentation?

I've amended my commit to remove increment! and decrement! from the list of methods that trigger callbacks in the guides, since they no longer do so.

@pas256
Copy link

pas256 commented Sep 4, 2016

I just ran into this issue of increment! not calling after_save.

I prefer the convenience of the old functionality but am fine if the new functionality (not calling callbacks) is documented.

@kamipo
Copy link
Member

kamipo commented Jan 3, 2017

r? @kaspth

@rafaelfranca rafaelfranca assigned kaspth and unassigned sgrif Jan 4, 2017
@kaspth kaspth merged commit 7f19f30 into rails:master Jan 8, 2017
kaspth added a commit that referenced this pull request Jan 8, 2017
@kaspth
Copy link
Contributor

kaspth commented Jan 8, 2017

Thanks! I've backported this to 5-0-stable as well: cfd19c7

@eugeneius
Copy link
Member Author

Thank you @kaspth! ❤️

@kaspth
Copy link
Contributor

kaspth commented Jan 9, 2017

🤓❤

@pedromartinez
Copy link

This breaking change really hurt our application :(

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

9 participants