Fix: PHPCS Errors for PHP files #143
Conversation
1. Blank line after function. 2. Ignore WordPress.NamingConventions.ValidHookName.UseUnderscores because of backward compatibility
1. Spacing and equal line alignement issue. 2. Ignore object param and return type because assuming php greater than 7.1 also requrires php 7.4 as plugin description. 3. Change CamelCase to snake_case variables.
1. Spacing and alignment issue. 2. Naming convention 3. Multiline array comma issue
src/Container.php
Outdated
| @@ -196,6 +196,6 @@ public function define_services(): void { | |||
| * | |||
| * @since 1.0.0 | |||
| */ | |||
| do_action( 'rtcamp.google_login_services', $this ); | |||
| do_action( 'rtcamp.google_login_services', $this ); // phpcs:ignore WordPress.NamingConventions.ValidHookName.UseUnderscores -- Ignore as currently cannot change because of backward compatibility. | |||
There was a problem hiding this comment.
What about disabling this rule in phpcs config?
<rule ref="WordPress-Core">
<exclude name="Generic.Arrays.DisallowShortArraySyntax" />
<exclude name="Generic.Commenting.DocComment.MissingShort" />
<exclude name="WordPress.PHP.DisallowShortTernary" />
+ <exclude name="WordPress.NamingConventions.ValidHookName.UseUnderscores" />
</rule>Probably include a todo comment as well?
<!-- @todo: Remove this once we rename the hooks with underscores and deprecate the old ones. -->There was a problem hiding this comment.
Yes, We can do exclude it from there with a todo for future, works well. 👍
|
@hbhalodia PHPUnit tests seem to be failing as there is Below diff should fix the issue. diff --git a/.github/workflows/phpunit_on_pull_request.yml b/.github/workflows/phpunit_on_pull_request.yml
index fcca561..e2670d3 100644
--- a/.github/workflows/phpunit_on_pull_request.yml
+++ b/.github/workflows/phpunit_on_pull_request.yml
@@ -48,9 +48,13 @@ jobs:
if: steps.check_files.outputs.files_exists == 'true'
run: echo "::add-matcher::${{ runner.tool_cache }}/phpunit.json"
+ - name: Update composer packages
+ if: steps.check_files.outputs.files_exists == 'true'
+ run: composer update
+
- name: Run PHPUnit
if: steps.check_files.outputs.files_exists == 'true'
- run: composer update && composer tests:unit
+ run: vendor/bin/phpunit tests/php/Unit/ --verbose
- name: Archive code coverage results
uses: actions/upload-artifact@v2
|
|
Hi @thelovekesh, Seems like some of the tests aer giving errors due to changes in path for the assets.
as we have not added build files folder and seems like in enquing this scripts and styles we need to change a code bit there to achieve what we need and then have to check the test. Other error is use of Thanks. |
|
@hbhalodia I think we can ignore the tests here for now since changes are going to be merged on a develop(refactor/v2) branch. |
There was a problem hiding this comment.
Thanks, @hbhalodia!
Note: Ignore failing tests as they will be fixed and reviewed before getting merged into the main branch.
* Fix: phpcs errors 1. Blank line after function. 2. Ignore WordPress.NamingConventions.ValidHookName.UseUnderscores because of backward compatibility * Fix: phpcs error for equal sign alignment * Fix: phpcs errors 1. Spacing and equal line alignement issue. 2. Ignore object param and return type because assuming php greater than 7.1 also requrires php 7.4 as plugin description. 3. Change CamelCase to snake_case variables. * Fix: phpcs error for file /tests/bootstrap.php for spacing issue * Fix: phpcs errors 1. Spacing and alignment issue. 2. Naming convention 3. Multiline array comma issue * Fix issue added by bot for undefined variable * Exclude hook name validation rule instead ignoring * Update unit tests workflow * Add slah before wp_json_encode function * Revert json encode change
HooksNaming convention we have ignored the error using comment because hooks can be used by other who have downloaded this plugin, so to add backward compatible we have untouched hooks.7.1as plugin usesobjecttype in param and return type.Issue - #142