-
Notifications
You must be signed in to change notification settings - Fork 0
chore: rescaffold plugin #27
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
Conversation
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.
Pull request overview
This PR rescaffolds the plugin to align with modern WordPress plugin development best practices, including updated tooling, configurations, and build processes.
- Updated build configuration from nested
assets/build/jsandassets/build/cssstructure to a flatbuild/directory - Implemented comprehensive testing infrastructure with PHPUnit, PHPStan, and enhanced PHPCS rulesets
- Added GitHub Actions workflows for automated testing and releases
Reviewed changes
Copilot reviewed 29 out of 38 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| webpack.config.js | Simplified output paths from nested structure to flat build/ directory |
| tests/bootstrap.php | Added PHPUnit test bootstrapper |
| phpunit.xml.dist | Added PHPUnit configuration |
| phpstan.neon.dist | Added PHPStan static analysis configuration |
| phpcs.xml.dist | Expanded PHPCS ruleset with VIP, Slevomat, and Plugin Check standards |
| package.json | Updated dependencies, scripts, and added TypeScript support |
| composer.json | Enhanced with PHPStan, additional coding standards, and test dependencies |
| onedesign.php | Refactored plugin loader with namespace and custom Autoloader wrapper |
| inc/Autoloader.php | Added custom autoloader wrapper with graceful error handling |
| inc/classes/class-assets.php | Updated asset paths to match new build structure |
| docs/* | Updated development documentation with comprehensive setup instructions |
| .github/workflows/* | Added automated testing and release workflows |
| Configuration files | Updated ignore patterns and editor configurations |
Comments suppressed due to low confidence (1)
phpcs.xml.dist:1
- The pattern 'node_modules (?)' contains an unusual '(?)' suffix. This should likely be 'node_modules/**' or just 'node_modules'.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
…n permissions Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
…n permissions Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
up1512001
left a comment
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.
@justlevine Added few doubts/feedbacks.
| cancel-in-progress: true | ||
|
|
||
| on: | ||
| release: |
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.
can we do this on tag instead of release so that we can directly generate plugin zip with dev tags also from our branch for efficient QA?
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.
What about it makes QA more efficient?
(In general, this plugin is just POC, so I'd much rather an intentional GH-hosted release process and only create a tag on GH release instead of first creating tags > deriving the release > releasing. )
| # - Runs PHPUnit tests (with coverage if enabled). | ||
| # - Uploads code coverage report to Codecov.io (if coverage is enabled). | ||
| # - Uploads HTML coverage report as an artifact (if coverage is enabled). | ||
| phpunit: |
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.
not sure for this as in any module we are not writing unit test cases so in future are we planning to add unit test cases?
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.
If this goes from POC to plugin we will.
Also, my bigger intent for this entire PR is using this (and the other One* plugins) as boilerplate for future plugins. Unit tests are only optional for direct client work, never again for publicly released plugins.
| $error_message = __( 'OneDesign: The Composer autoloader was not found. If you installed the plugin from the GitHub source code, make sure to run `composer install`.', 'onedesign' ); | ||
|
|
||
| if ( defined( 'WP_DEBUG' ) && WP_DEBUG ) { | ||
| error_log( esc_html( $error_message ) ); // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_error_log -- This is a development notice. |
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.
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.
Thanks for surfacing that discussion🙇
Indeed the pattern is fine, and is used by several canonical and noncanonical plugins.
| "uninstall.php" | ||
| ], | ||
| "dependencies": { | ||
| "@wordpress/api-fetch": "^7.35.0", |
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.
did we tested plugin with updated dependencies, because I think that should be separate part. 🤔
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.
Yes they've been tested and confirmed compatible.
(Also we don't bundle the @wordpress dependencies, so in general nonbreaking changes are safe, and breaking ones you just need to confirm that we're not relying on anything that was removed)
Co-authored-by: Utsav Patel <[email protected]>
What
This PR rescaffolds plugin boilerplate to follow latest plugin best practices.
Includes:
Why
Related Issue(s):
How
This PR does not include any formatting/remediations from fixing the linting.
Testing Instructions
Screenshots
Additional Info
Checklist