-
-
Notifications
You must be signed in to change notification settings - Fork 10.2k
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
Don't populate settings defaults twice on startup #7121
Don't populate settings defaults twice on startup #7121
Conversation
This change causes the default settings to only be populated if there's a migration, and not in every other case? |
Wait no, the way the code was collapsed was super confusing. |
So this is good? |
Oops, seems to be a bit of a test bug 😛 |
i don't think this fix solves the underlying problem. |
Fair enough. I feel like removing the first settings populate call is still necessary though. |
i think we need the call, because
|
@acburdine it seems there's a test failure with the re-created branch 😞 |
5fdaf0f
to
cc3f0cc
Compare
@kevinansfield fixed it |
Slight regression that throws an error whenever a new setting is added. (only on the first boot of the blog, doesn't happen after that).
To reproduce the error (without this PR):
With these changes that doesn't happen.