Skip to content

Conversation

@schlessera
Copy link
Member

This PR makes changes to the package structure to fit the one from the bundled plugins:

  • Use WP_CLI\Doctor as the namespace root
  • Use PSR-4 autoloader
  • Use latest wp-cli-tests framework with Behat v3

@schlessera schlessera requested a review from a team as a code owner January 13, 2022 03:20
@schlessera schlessera added this to the 2.1.1 milestone Jan 13, 2022
@danielbachhuber danielbachhuber removed this from the 2.1.1 milestone Aug 19, 2022
@danielbachhuber
Copy link
Member

@schlessera What do you think about punting on the namespace change, and simply applying the safe bits?

@swissspidy swissspidy requested a review from a team November 3, 2023 13:11
@swissspidy
Copy link
Member

@schlessera @danielbachhuber Apologies for the noise here, but I thought I'd pick this up and help fix all the failing tests. This is now ready for review!

I don't mind the namespace change, I think it should be fine. Just noting that this handbook page needs updating after merge: https://github.com/wp-cli/handbook/blob/7c792d2a11448dab77f23d8055f82553e6dff2a6/doctor-customize-config.md

@swissspidy swissspidy added this to the 2.2.0 milestone Nov 3, 2023
Copy link
Member

@danielbachhuber danielbachhuber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really a fan of this pull request as it currently is. It will cause a lot of breakage for existing users, for very little real-world benefit.

If we're committed to renaming the classes, then I think we should add backcompat for the existing classes (and make sure there's backcompat for the doctor.yml file).

@swissspidy
Copy link
Member

Makes sense.

Btw, any idea why this one test is failing on 6.2?
The 3.7 one I haven‘t looked into yet

@danielbachhuber
Copy link
Member

Btw, any idea why this one test is failing on 6.2?

$ wp core version
6.2
$ wp core check-update
+---------+-------------+-----------------------------------------------------------------------+
| version | update_type | package_url                                                           |
+---------+-------------+-----------------------------------------------------------------------+
| 6.2.3   | minor       | https://downloads.wordpress.org/release/wordpress-6.2.3-partial-0.zip |
| 6.3.2   | major       | https://downloads.wordpress.org/release/wordpress-6.3.2.zip           |
+---------+-------------+-----------------------------------------------------------------------+
$ wp doctor check core-update
+-------------+--------+----------------------------------------------------------------------+
| name        | status | message                                                              |
+-------------+--------+----------------------------------------------------------------------+
| core-update | error  | Updating to WordPress' newest minor version is strongly recommended. |
+-------------+--------+----------------------------------------------------------------------+
Error: 1 check reports 'error'.

It looks like main is passing because behat.yml isn't detected and no tests are being run.

@swissspidy
Copy link
Member

Yes, that I realized. That‘s why I did all these commits to fix tests. Just thought it was weird that it failed for that particular job. But I can adapt the test 👍

@swissspidy
Copy link
Member

OK looks like @require-wp-latest doesn't seem to actually work 🤔

@danielbachhuber
Copy link
Member

OK looks like @require-wp-latest doesn't seem to actually work 🤔

@swissspidy Lovely. Maybe it's not actually a thing!

For Scenario: WordPress is up to date, you could run wp core update prior to wp doctor check core-update. I'd probably do the same for Scenario: Use --spotlight to focus on warnings and errors.

@swissspidy
Copy link
Member

Thanks, yeah I‘ll try that again.

Amd then I guess I‘ll review all existing usage of @require-wp-latest in the code base 🤔

Any idea on the WP 3.7 test? Can‘t run them locally as I don‘t have old PHP installed.

@danielbachhuber
Copy link
Member

Any idea on the WP 3.7 test? Can‘t run them locally as I don‘t have old PHP installed.

Nothing immediately obvious, no.

@swissspidy
Copy link
Member

Now there are some random errors from extension command as well, like Warning: twentyeleven: version higher than expected.. Core bumped their versions yesterday
but they haven't been released yet in the theme directory. So probably should go away after the 6.4 release tomorrow 🤷

I'll leave this PR be for now, maybe someone else wants to pick it up.

@swissspidy
Copy link
Member

@ernilambar Since you worked on this repo recently, maybe this PR is something you'd be interested in bringing over the finish line?

@ernilambar
Copy link
Member

@ernilambar Since you worked on this repo recently, maybe this PR is something you'd be interested in bringing over the finish line?

Sure. I will work on it.

@ernilambar
Copy link
Member

Closing in favor of #194

@ernilambar ernilambar closed this Jul 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants