(Preferences)(hotfix)(17.5) Hotfix for missing preferences in the core scope
#58031
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…a deprecation message (#58016)
…rom the original get is `undefined`
|
Size Change: +317 B (0%) Total Size: 1.69 MB
ℹ️ View Unchanged
|
|
So if I'm understanding properly, this is just the same code that went into trunk but the list of migrated settings is not entirely the same. What settings are not in 7.5? Regardless, this makes sense to me though. |
fullofcaffeine
changed the title
Yep! I don't recall from the top of my head what settings are missing :/ I think this would be safer than checking what preference migration PRs are missing from the 17.5 branch. |
|
The test failures are related to the deprecation warnings introduced in this PR. They are not being expected by the tests, but they should be benign failures AFAIU @youknowriad |
|
@youknowriad If you agree with these changes, would you mind approving here? Thanks! |
|
All failures here seem benign, related to tests not expecting the new deprecation warning console messages. The Playwright E2Es are all passing, except Puppeteer, which fails because of the unexpected warnings. I'll merge this and bake a 17.5.1 release with this fix. |
fullofcaffeine
changed the title
core scope
fullofcaffeine
deleted the
hotfix/for-missing-dependencies-in-core-17.5
branch
fullofcaffeine
added
the
[Type] Bug
fullofcaffeine
added a commit
that referenced
this pull request
Jan 23, 2024
… migrated to the `core` scope, but will still work due to the 17.5.1-specific hotfix 17.5.1-specific hotfix: #58031.
This was referenced Jan 23, 2024
Closed
Closed
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
17.5-specific follow-up to #58016, which fixes the issue on
trunkbut causes a BSOD in therelease/17.5branch. The problem is that not all changesets that migrated the preferences in thesettingsToMoveToCoredefined in #58016 are in therelease/17.5branch.One solution would be to cherry-pick the missing changesets. Still, I'm not sure if there could be other consequences (i.e if those changesets depend on other changesets that are supposed to be released with 17.6), so I've come up with this hotfix that should be merged only to the
release/17.5branch.Why?
After the fix in #58016, I wanted to ship a 17.5.1 with the fix, but after manually smoke testing the draft, I found out it caused a BSOD in the site editor:
Screen.Recording.2024-01-19.at.13.25.51.mov
Further investigation led to my explanation under the "What" section.
How?
It extends the on proxy added in #58016 to fallback to the passed
scopeif the value returned from theoriginalGetisundefinedfor the preferences in thesettingsToMoveToCorearray.Testing Instructions
Test that it doesn't break a plugin consumer of one of the preferences
Let's use Yoast as it was the plugin that had the issue originally (see Automattic/wp-calypso#86601 and #57988).