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

Normalize resetting schema cache version for YAML and Marshal #38564

Merged
merged 1 commit into from
Feb 24, 2020
Merged

Normalize resetting schema cache version for YAML and Marshal #38564

merged 1 commit into from
Feb 24, 2020

Conversation

kytrinyx
Copy link
Contributor

Extract a method to reset the schema cache to make serialization more consistent between YAML and Marshal.

Summary

When dumping the schema cache, we first call clear! which resets
everything, including setting the @Version to nil.

Dumping with YAML dumps the correct version, but doesn't actually
update the instance variable.

Dumping with Marshal writes the variable.

This change makes it more consistent between the two strategies.

Also, for codebases (such as GitHub) that use an alternate versioning strategy for the schema cache, they only need to overwrite reset_version! rather than overwriting the serialization method.

When dumping the schema cache, we first call clear! which resets
everything, including setting the @Version to nil.

Dumping with YAML dumps the correct version, but doesn't actually
update the instance variable.

Dumping with Marshal writes the variable.

This change makes it more consistent between the two strategies.
Also, for codebases that use an alternate versioning strategy,
they only need to overwrite reset_version! rather than overwriting
the serialization method.
@rafaelfranca rafaelfranca merged commit 69d25a5 into rails:master Feb 24, 2020
@kytrinyx kytrinyx deleted the normalize-schema-cache-version-reset branch February 24, 2020 20:06
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

2 participants