Integration of Composer and PHP CodeSniffer for Improved Standards Compliance #24
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Integrate Composer for dependency management and PHP CodeSniffer for code quality checks, targeting PHP versions 7.3 to 8.3. Includes updates to .gitignore, dependabot.yml, and introduces lint_phpcs.yml for automated linting with PHP CodeSniffer. A custom phpcs.xml file is added to enforce coding standards tailored to WordPress.
Refactor various PHP classes to enhance and standardize WooCommerce query handling. Introduce consistent formatting and optimize performance by refining query filters and hooks.
|
The recent commit was the result of using phpcbf to automatically fix coding standards issues. Here’s a summary of the corrections: I don’t believe this should break anything, but just in case, could you please take a look? It fixed a number of issues, but the remaining ones will need to be addressed manually. |
|
For what remains to be resolved (This comes from the CI of my repo, but the results would be the same when this pull request is accepted). |
MaximeMichaud
commented
Sep 28, 2024
| @@ -296,16 +296,16 @@ public function trigram_clause( $value ) { | |||
| // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared | |||
| return $wpdb->prepare( 'SELECT DISTINCT i FROM ' . $this->tablename . ' WHERE t = %s', $trigrams[0] ); | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
299 | ERROR | Empty line required before block comment
| | (Squiz.Commenting.BlockComment.NoEmptyLineBefore)
MaximeMichaud
commented
Sep 28, 2024
| @@ -371,16 +375,14 @@ public function update( array $order_ids ) { | |||
| } | |||
|
|
|||
| if ( $this->is_ready() ) { | |||
| /* | |||
| Do this all at once to avoid autocommit overhead. */ | |||
| // Do this all at once to avoid autocommit overhead. | |||
There was a problem hiding this comment.
375 | ERROR | Comment closer must be on a new line
| | (Squiz.Commenting.BlockComment.CloserSameLine)
MaximeMichaud
commented
Sep 28, 2024
| // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching | ||
| $wpdb->query( 'BEGIN;' ); | ||
| foreach ( $order_ids as $order_id ) { | ||
| /* | ||
| Get rid of old metadata */ | ||
| // Get rid of old metadata |
There was a problem hiding this comment.
380 | ERROR | Comment closer must be on a new line
| | (Squiz.Commenting.BlockComment.CloserSameLine)
MaximeMichaud
commented
Sep 28, 2024
| // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared,WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching | ||
| $wpdb->query( $wpdb->prepare( 'DELETE FROM ' . $tablename . ' WHERE i = %d', $order_id ) ); | ||
| /* Retrieve and add the new metadata */ | ||
| // Retrieve and add the new metadata |
There was a problem hiding this comment.
Squiz.Commenting.BlockComment.CloserSameLine
MaximeMichaud
commented
Sep 28, 2024
There was a problem hiding this comment.
Method name "getInstance" is not in snake case format, try "get_instance"
(WordPress.NamingConventions.ValidFunctionName.MethodNameInvalid)
There was a problem hiding this comment.
Variable "$newQuery" is not in valid snake_case format, try "$new_query"
(WordPress.NamingConventions.ValidVariableName.VariableNotSnakeCase)
|
I have other changes, but I will wait before committing, and I will test the modified version of the plugin in production. |
|
All the code that was modified previously has been in production since yesterday, and I checked my debug.log file, which has been active since the beginning of September. It seems that there are no issues at all. However, I noticed a log entry from a few weeks ago. I haven't encountered it again since, but I'm mentioning it here just so you're aware of it: This error is not related to this pull request, but I'm mentioning it just in case :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hello,
This pull request represents a more substantial update compared to my last submission. I have integrated Composer, which I had only used sparingly before, but I have learned a lot about it in the past few hours. After reviewing several popular plugins, I decided to implement Composer for this plugin as well.
In this pull request, not everything is perfect, and there may be adjustments needed in the long term, but for now, it seems adequate.
Key Changes:
PHP CodeSniffer Integration:
phpcs.xmlfile. This configuration is not overly strict and is aligned with WordPress coding standards. Please review it to determine if you'd like to adjust the strictness.Composer Configuration:
composer.jsonfile has been expanded to include more than what is currently necessary, foreseeing future needs. This should not cause any issues at present..gitignoreto properly exclude thevendordirectory and included thecomposer.lockfile to ensure consistent dependency versions across all setups.Dependency Management:
dependabot.ymlconfiguration for Composer to regularly check and update dependencies, ensuring optimal and secure operation.Linting Workflow:
lint_phpcs.yml, utilizing a matrix strategy to check PHP versions from 7.3 to 8.3. It's worth noting that versions below PHP 7.3 do not support the dependencies, and PHP 7.3 has been nearing its end-of-life for three years now (PHP End of Life Dates). It may be beneficial to increase the minimum PHP version supported by the plugin, but it currently functions well across all specified versions.<rule ref="PHPCompatibility"/><config name="testVersion" value="5.6-"/>This setup has taken several hours to implement and refine. Feel free to make any adjustments to the
composer.jsonif you have specific preferences or requirements.Should you accept this pull request, further code modifications may be necessary, or you might opt to tweak the
phpcs.xml, though that could be considered a workaround depending on the context :)I look forward to your feedback and am happy to make any further adjustments as needed.