Fix the issues reported by phpcs

Hi @ urvashi_vora, I have applied your patch and run successfully .

These are the steps I followed:

1. Took clone in drupal 10.1.x

2. Ran this command:

./vendor/bin/phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,m
d,yml,twig modules/contrib/shopify_app/

3. Applied patch and again ran phpcs command.

found errors.

I have fixed that error with phpcbf.

Ran this command to fix the errors:

./vendor/bin/phpcbf --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,m
d,yml,twig modules/contrib/shopify_app/

4. Then again checked with phpcs:

./vendor/bin/phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,m
d,yml,twig modules/contrib/shopify_app/

Found no errors.

Needs review.

-          // Not an app proxy request, possible a current session already exists?
+          // Not app proxy request, possible a current session already exists?

The indefinite article is correct. If a line is longer than 80 characters, it just need to be split in two.
Anyway, that sentence is not grammatically correct and it should be rewritten. For example, it must be possibly, a current session already exists.

   /**
-   * constructor.
+   * Constructor.
    *
    * @param \Drupal\shopify_app\Authentication\ImpersonateShopifyRequest $impersonateShopifyRequest
-   *   The authentication provider
+   *   The authentication provider.
    */

For constructors, the documentation comment is not anymore necessary. If it is added, the description should be Constructs a new [class name] object. where [class name] is the class name, namespace included.

Please review.

Hi @sakthi_dev
Your patch #4 is fixing some of the issues of #3 but * @param \Drupal\shopify_app\Authentication\ImpersonateShopifyRequest $impersonateShopifyRequest this line is still longer than 80 characters .
Please check

I have made some changes as mentioned above.
Please review my patch.

+          // Not an app proxy request, it must be possible, a current session
+          // already exists.

That is a comma-split sentence. It is not an proxy request. It is possible that a current session already exists. is better.

-   * constructor.
+   * Constructs a new RequestPolicy Object.

The class namespace is still missing. object is spelled in lowercase letters, since it is not at the beginning of the description.

Hi apaderno,
I have fixed errors in this patch as you mentioned.
Please review.

Hi @aayushamkotia,

I applied patch #9, it applied successfully but still resulted to 1 error.

shopify_app git:(1.0.x) curl https://www.drupal.org/files/issues/2023-09-13/comment_errors_fixed_3372412_9.patch | patch -p1
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  4493  100  4493    0     0  17969      0 --:--:-- --:--:-- --:--:-- 18878
patching file README.txt
patching file config/schema/shopify_app.schema.yml
patching file shopify_app.install
patching file shopify_app.module
patching file src/Access/AppAccessCheck.php
patching file src/Authentication/ImpersonateShopifyRequest.php
patching file src/PageCache/RequestPolicy.php
patching file src/ShopInterface.php
patching file src/ShopifyAppPermissions.php
➜  shopify_app git:(1.0.x) ✗ cd ..
➜  contrib git:(main) ✗ phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig shopify_app

FILE: /Users/PrometInterns/Demo-site/drupal-orgissue/web/modules/contrib/shopify_app/src/Access/AppAccessCheck.php
------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------------------------------
 7 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Core\Routing\Access\AccessInterface.
------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------------------------------------------------------

Time: 440ms; Memory: 10MB

Kindly check

Thanks,
Jake

Hi @apaderno,

Applied your latest changes, it threw more errors than the patch #9

shopify_app git:(1.0.x) curl https://git.drupalcode.org/project/shopify_app/-/merge_requests/1.diff | patch -p1
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  8408    0  8408    0     0  21810      0 --:--:-- --:--:-- --:--:-- 22361
patching file .gitlab-ci.yml
patching file shopify_app.install
patching file shopify_app.module
patching file src/Access/AppAccessCheck.php
patching file src/Authentication/ImpersonateShopifyRequest.php
patching file src/PageCache/RequestPolicy.php
patching file src/ShopInterface.php
patching file src/ShopifyAppPermissions.php
➜  shopify_app git:(1.0.x) ✗ cd ..
➜  contrib git:(main) ✗ phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig shopify_app

FILE: /Users/PrometInterns/Demo-site/drupal-orgissue/web/modules/contrib/shopify_app/config/schema/shopify_app.schema.yml
-------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------------------------
 16 | ERROR | [x] Expected 1 newline at end of file; 0 found
-------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-------------------------------------------------------------------------------------------------------------------------


FILE: /Users/PrometInterns/Demo-site/drupal-orgissue/web/modules/contrib/shopify_app/README.txt
-----------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-----------------------------------------------------------------------------------------------
 1 | ERROR | [x] Expected 1 newline at end of file; 0 found
-----------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-----------------------------------------------------------------------------------------------


FILE: /Users/PrometInterns/Demo-site/drupal-orgissue/web/modules/contrib/shopify_app/src/Access/AppAccessCheck.php
-------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AND 1 WARNING AFFECTING 2 LINES
-------------------------------------------------------------------------------------------------------------------------------------
 12 | ERROR   | [x] Use statements should be sorted alphabetically. The first wrong one is Symfony\Component\HttpFoundation\Request.
 61 | WARNING | [ ] Line exceeds 80 characters; contains 81 characters
-------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-------------------------------------------------------------------------------------------------------------------------------------

Time: 604ms; Memory: 10MB

Kindly check and advise.

Thanks,
Jake

silvi.addweb → made their first commit to this issue’s fork.

Fixed all the following issues. Please review.

I have reviewed and applied patch cleanly, it resolved all the phpcs issues. (see attached screenshot)