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

Properly support @require-wp-latest #190

Closed
2 tasks done
swissspidy opened this issue Nov 7, 2023 · 4 comments · Fixed by wp-cli/language-command#135
Closed
2 tasks done

Properly support @require-wp-latest #190

swissspidy opened this issue Nov 7, 2023 · 4 comments · Fixed by wp-cli/language-command#135

Comments

@swissspidy
Copy link
Member

Bug Report

Describe the current, buggy behavior

I spotted a few instances of @require-wp-latest in our code base, but I don't think it actually works. At least that wasn't the case when I tried it at wp-cli/doctor-command#165

run-behat-tests already grabs the latest version of WordPress, so when we do things like WP_VERSION=6.2 composer behat, it could pass this info on to behat-tags.php so that the script knows that 6.2 is lower than the current latest.

Describe what you would expect as the correct outcome

@require-wp-latest works as expected

Provide a possible solution

Provide additional context/Screenshots

@wojsmol
Copy link
Contributor

wojsmol commented Nov 7, 2023

@swissspidy This is perpetually skipped - see

// Only apply @require-wp tags when WP_VERSION isn't 'latest', 'nightly' or 'trunk'.
// 'latest', 'nightly' and 'trunk' are expected to work with all features.
if ( $wp_version &&
! in_array( $wp_version, array( 'latest', 'nightly', 'trunk' ), true ) ) {
$wp_version_reqs = array_merge(
version_tags( 'require-wp', $wp_version, '<', $features_folder ),
version_tags( 'less-than-wp', $wp_version, '>=', $features_folder )
);
} else {
// But make sure @less-than-wp tags always exist for those special cases. (Note: @less-than-wp-latest etc won't work and shouldn't be used).
$wp_version_reqs = array_merge(
$wp_version_reqs,
version_tags( 'less-than-wp', '9999', '>=', $features_folder )
);
}

@swissspidy
Copy link
Member Author

Yeah I saw that code too. The thing is:

  1. We still use @require-wp-latest in many tests, despite it not working
  2. We can actually make it work

@swissspidy swissspidy changed the title @require-wp-latest does not seem to work Properly support @require-wp-latest Nov 7, 2023
@danielbachhuber
Copy link
Member

I'd suggest we actually remove @require-wp-latest. It's ambiguous, and I don't think it's entirely necessary.

We still use @require-wp-latest in many tests, despite it not working

I only see one use: https://github.com/search?q=org%3Awp-cli%20require-wp-latest&type=code

Are there others?

@swissspidy
Copy link
Member Author

I only see one use: github.com/search?q=org%3Awp-cli%20require-wp-latest&type=code

Are there others?

I think you're right! I just double checked and I just saw tons of duplicates in all the different vendor folders, so yeah just that one.

I suppose it not even working shows how unnecessary it can be in that test there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants