-
Notifications
You must be signed in to change notification settings - Fork 4.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
Make wp-env compatible with WordPress versions older than 5.4 by fixing wp-config anchors #55864
Conversation
…ng wp-config anchors.
Size Change: 0 B Total Size: 1.77 MB ℹ️ View Unchanged
|
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.
As mentioned in one of the comments, could we try using --anchor=EOF
instead? Since wp config set
does both updates and additions, I think that would work out pretty well and avoid the version checking altogether.
@ObliviousHarmony Based on the reason why the |
The anchor defaults to
|
@swissspidy I thought you said in #55864 (comment) that these were no longer being translated. Or was that just very recent? Maybe I misunderstood that comment. |
Just very recent. But WP 5.1 and WP 5.4 still had translated config files in many locales. |
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
@swissspidy @ObliviousHarmony It's been a while, but I just updated this PR based on previous feedback and left another reply to the other concern. I still think this would be valuable to land as the underlying problem still persists. Would be great to get another review! |
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.
Do we really want to support such ancient WordPress versions?
There are no issues filed for this either, so wondering if it's really worth it.
Unfortunately I don't have Docker and therefore can't test it, so I'll leave the actual approval to others.
I confirmed that this PR works on a Windows host OS.
I too am a bit skeptical about the need to support such old WordPress versions. I'd be interested to hear other people's opinions on this too. |
@swissspidy @t-hamano Thanks for the review and testing! I get the general concern about supporting old WordPress versions, but I find it a bit of a moot point, for a few reasons:
I think the cost of supporting old versions is minimal (basically this PR), and with the foundation that (FWIW I personally have one of those simple plugins where I still support back to 5.0 because there's no need to require anything higher. That's how I ran into this bug.) |
Flaky tests detected in 1be7b01. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/10394400926
|
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.
Minus the remaining console.log
What?
@wordpress/env
currently does not support WordPress versions older than 5.4.Why?
@wordpress/env
currently assumes certain conventions inwp-config.php
that were not the case prior to WordPress 5.4.@wordpress/env
does:wp-config.php
was not using spacing correctly: Compare the 5.1 code with the 5.0 code for reference.wp-config.php
was using the olderdirname( __FILE__ )
instead of__DIR__
: Compare the 5.4 code with the 5.3 code for reference.How?
wp-config.php
anchor references depending on which WordPress version is used.Testing Instructions
@wordpress/env
with different WordPress ZIP files in the.wp-env.json
file under"core"
:Error: Could not process the 'wp-config.php' transformation.
ABSPATH
won't have been made. This will for instance prevent PHPUnit tests from working correctly.