-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Set autoload to 'no' for previously activated themes using upgrade routine #5711
Conversation
@swissspidy i get |
No idea why this happens just for this PR 🤷 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The upgrade routine looks good. I don't see how the Performance Test failures would be related to this PR, so let's wait to see if it effects other PRs and fix that issue separately.
Seems confirmed that the Performance Test is failing due to this change for some reason as other, more recent runs, are not effected. See: https://github.com/WordPress/wordpress-develop/actions/workflows/performance.yml?query= We need to debug this before committing in order to not introduce failures into trunk. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the PR @mukeshpanchal27. It mostly looks good, but there is one crucial thing missing I believe.
The Performance tests going fails if we update DB version. Check PR: #5716 not sure but seems there is bug Performance tests setup. |
Oooh I know what's up! Because of the version bump and the following checkout of the previous commit, WordPress is showing the "Database upgrade required" screen, so you first have to run the db upgrade. I tested this hypothesis in #5717, where you can see that tests now pass :) |
Great @swissspidy, it's fascinating how we can encounter unforeseen issues while focusing on different tasks. I suggest we commit that PR initially without altering the DB version. Afterward, we can rebase this PR, making it ready for the final commit. |
Performance tests ✅ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @mukeshpanchal27, LGTM!
Committed in https://core.trac.wordpress.org/changeset/57155 |
Trac ticket: https://core.trac.wordpress.org/ticket/59975
This PR add upgrade routine for 6.5 that set autoload to 'no' for previously activated themes.
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.