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

README: make it clear that the current stable version (v2.3.0) does not work on PHP8 #2219

Closed
wants to merge 1 commit into from

Conversation

bluefuton
Copy link

I attempted to install PHPCS and WPCS on a new machine running PHP 8.0.28 today, and spent quite a bit of time researching why it wasn't working and what the appropriate remedy was.

Specifically I ran into this bug: #1967 and it doesn't appear that the fix for this will be released until v3.0.0.

In the meantime, it would be kind to developers if the project makes it clear upfront that PHP8 is not supported, and recommends PHP 7.4 instead.

@jrfnl
Copy link
Member

jrfnl commented Apr 11, 2023

@bluefuton Thank you for your willingness to contribute to WordPressCS.

While I appreciate what you are trying to do, making this change in the develop branch is not the right place as the README for develop applies to the upcoming 3.0.0 version and that version does work with PHP 8.0+.

@jrfnl
Copy link
Member

jrfnl commented Apr 11, 2023

As there have been several issues about this, including issues about how to install WPCS develop, it might be an idea to "pin" one of those issues to top of the issue page. What do you think ?

If you think that would help, suggestions on which issue would be the best one to pin would be welcome.

@bluefuton
Copy link
Author

While I appreciate what you are trying to do, making this change in the develop branch is not the right place as the README for develop applies to the upcoming 3.0.0 version and that version does work with PHP 8.0+.

Thanks @jrfnl! I did consider this, but the README for develop is the first thing users will see when they visit the project on Github. Could I try adjusting the text so it remains appropriate for v3.0.0?

As there have been several issues about this, including issues about how to install WPCS develop, it might be an idea to "pin" one of those issues to top of the issue page. What do you think ?

Possibly #1967, as that's where I ended up - or creating a new issue called "PHP8 compatibility" summarizing the current situation?

My problem solving journey was:

  1. Search error message: phpcs vsprintf(): Argument #2 ($values) must be of type array, string given
  2. Landed on this helpful issue in Human Made's repo: Uncaught TypeError: vsprintf(): Argument #2 ($values) must be of type array, string given humanmade/coding-standards#278
  3. Found the PR fixing the issue PHP 8 | ControlStructureSpacing: bug fix - $data should be an array #1935
  4. Discovered that the PR hadn't been released yet
  5. Went looking for related issues in the repo, found PHP 8.0 vsprintf #1967
  6. Tried to downgrade to PHP 7.4 but Homebrew wouldn't allow it because it's unsupported. Found that there were ways around that, but decided not to continue faffing with Homebrew.
  7. Manually replaced WordPress/Sniffs/WhiteSpace/ControlStructureSpacingSniff.php with the latest version from develop, and everything works for the moment :)

Thanks for all your hard work on v3.0.0! Sounds like it has been a big effort to get to PHP 8 compatibility.

@@ -58,7 +58,7 @@ This project is a collection of [PHP_CodeSniffer](https://github.com/squizlabs/P

### Requirements

The WordPress Coding Standards require PHP 5.4 or higher and [PHP_CodeSniffer](https://github.com/squizlabs/PHP_CodeSniffer) version **3.7.2** or higher.
The WordPress Coding Standards require PHP 5.4 or higher and [PHP_CodeSniffer](https://github.com/squizlabs/PHP_CodeSniffer) version **3.7.2** or higher. The current stable release does not work on PHP 8, and for the moment we recommend that you use PHP 7.4.
Copy link
Author

Choose a reason for hiding this comment

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

Suggested change
The WordPress Coding Standards require PHP 5.4 or higher and [PHP_CodeSniffer](https://github.com/squizlabs/PHP_CodeSniffer) version **3.7.2** or higher. The current stable release does not work on PHP 8, and for the moment we recommend that you use PHP 7.4.
The WordPress Coding Standards require PHP 5.4 or higher and [PHP_CodeSniffer](https://github.com/squizlabs/PHP_CodeSniffer) version **3.7.2** or higher. PHP 8 is only supported in WordPress Coding Standards v3.0.0 and above.

@BWBama85
Copy link

As there have been several issues about this, including issues about how to install WPCS develop, it might be an idea to "pin" one of those issues to top of the issue page. What do you think ?

If you think that would help, suggestions on which issue would be the best one to pin would be welcome.

Where are the instructions on how to install WPCS develop? If you follow the instructions in the readme under develop it installs 2.3.0.

@dingo-d
Copy link
Member

dingo-d commented Apr 27, 2023

@BWBama85 In your project's composer.json file you can add

{
  "repositories": [
    {
      "type": "git",
      "url": "https://github.com/WordPress/WordPress-Coding-Standards.git"
    }
  ],
  "require-dev": {
    "wp-coding-standards/wpcs": "dev-develop"
  }
}

Then, running composer install will install the develop branch of WPCS.

Hope this helps.

@jrfnl
Copy link
Member

jrfnl commented Apr 27, 2023

AFAIK you shouldn't even need to add the repositories key in composer.json ;-)

Copy link

@U1F U1F left a comment

Choose a reason for hiding this comment

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

I think this makes perfect sense.

@GaryJones
Copy link
Member

Since WP 3.0 is close to release, I'm closing this one out.

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.

6 participants