Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[CMSP-669] Update pantheon wp coding standards #25

Merged
merged 8 commits into from
Sep 14, 2023

Conversation

jazzsequence
Copy link
Contributor

@jazzsequence jazzsequence commented Sep 13, 2023

  • Updates Pantheon Mu Plugin to use Pantheon WP Coding Standards 2.0
  • Fixes all the linting issues highlighted by the updated coding standards
  • Adds a PANTHEON_MU_PLUGIN_VERSION constant and actually updates the mu-plugin version (which hasn't been done at all since it was added afaict)

Todo

  • There's no actual linting happening on the repo, we should add GH actions to at least cover linting (and maybe testing later)
  • A new 1.2.0 release should be created after this is merged (we won't release until we know/understand how releases are handled via Updatinator and how the update will push out to pantheon-systems/WordPress).
  • we should add a contributing.md file since there are two places to bump versions now
  • we probably should add a .gitattributes file to exclude non-important files from releases (like the new phpcs.xml, GH actions, contributing, etc)
  • Prior to merge, we probably should make the changes in update-tool in case WP releases a patch tomorrow.

and add a PANTHEON_MU_PLUGIN_VERSION constant
not breaking this out into separate commits. this is a mix of phpcbf fixes and manual fixes to bring in line with Pantheon WP Coding Standards @2.0
@jazzsequence jazzsequence requested a review from a team as a code owner September 13, 2023 15:57
@guardrails
Copy link

guardrails bot commented Sep 13, 2023

⚠️ We detected 1 security issue in this pull request:

Insecure Processing of Data (1)
Severity Details Docs
Medium Title: Cross-Site Scripting with user input
echo wp_kses_post( sprintf( __( 'If you wish to update or add plugins using the WordPress UI, switch your site to SFTP mode from <a href="%s">your Pantheon dashboard</a>.', 'pantheon-systems' ), 'https://dashboard.pantheon.io/sites/' . $_ENV['PANTHEON_SITE'] ) );
📚

More info on how to fix Insecure Processing of Data in PHP.


👉 Go to the dashboard for detailed results.

📥 Happy? Share your feedback with us.

@jazzsequence
Copy link
Contributor Author

🤦 this was meant to be a draft...

@jazzsequence jazzsequence changed the title [CMSP-669] Update pantheon wp coding standards [CMSP-669] Update pantheon wp coding standards [wip] Sep 13, 2023
@jazzsequence
Copy link
Contributor Author

Note to future selves:

These lines in https://github.com/pantheon-systems/update-tool need to be updated to essentially mirror the .gitattributes file: https://github.com/pantheon-systems/update-tool/blob/master/src/Update/Filters/CopyMuPlugin.php#L55-L60

@jazzsequence
Copy link
Contributor Author

Alternately, we could specify a tag/release in the git path https://github.com/pantheon-systems/update-tool/blob/master/update-tool.yml#L53 but that would require updating Update Tool manually whenever we push a new version of the mu plugin.

@jazzsequence jazzsequence self-assigned this Sep 14, 2023
and exclude it from release packages
@jazzsequence jazzsequence changed the title [CMSP-669] Update pantheon wp coding standards [wip] [CMSP-669] Update pantheon wp coding standards Sep 14, 2023

Update Tool clones whatever the latest code on `main` is, and manually removes files that are not required for WordPress sites (e.g. `composer.json`, `composer.lock`, `README.md`, etc.). This is then bundled as part of WordPress releases and upstream updates.

Because the WordPress upstream is only updated when a new WordPress release is cut, it's less risky that we don't have an explicit `develop` branch, but `main` should still always be in a stable state in the chance that a WordPress bugfix or security release is pushed unexpectedly.
Copy link
Contributor

Choose a reason for hiding this comment

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

This feels better for readme than the workflow since it isn't actually a thing that happens, but that's a minor nitpick.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, but only contributors should really care about it. ¯_(ツ)_/¯

Copy link
Contributor

@rwagner00 rwagner00 left a comment

Choose a reason for hiding this comment

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

👍

* Author: Pantheon
* Author URI: https://pantheon.io/
*
* @package pantheon
*/

define( 'PANTHEON_MU_PLUGIN_VERSION', '1.2.0' );
Copy link
Member

Choose a reason for hiding this comment

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

  • I would put this in README as well
  • Why 1.2 and not 1.1 (I know this should have been incremented many times along the way)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I consider, since WPCS -> 3.0 is a big deal and we weren't linting at all previously, this to be a larger than patch update.

Copy link
Contributor Author

@jazzsequence jazzsequence Sep 14, 2023

Choose a reason for hiding this comment

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

I would put this in README as well

@pwtyler you mean list the version number in the readme? that would be one more thing we need to update when we cut new releases.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants