Fix the issues reported by phpcs

Issue created by @dineshkumarbollu
Issue was unassigned.
Status changed to Needs review over 1 year ago10:08am 25 April 2023
Comment over 1 year ago →
🇮🇳India dineshkumarbollu
Status changed to Needs work over 1 year ago5:05pm 25 April 2023
Comment over 1 year ago →
🇵🇭Philippines clarkssquared
Hi dineshkumarbollu,

I applied your patch#2 to the "viewer" module against version 1.0.1-beta1 and noticed that there are still remaining PHPCS issues being flagged.

Please look at the screenshot attached for your reference

Please review and advise,
Thanks
Status changed to Needs review over 1 year ago7:02am 26 April 2023
Comment over 1 year ago →
🇮🇳India mrinalini9 New Delhi
Updated patch #2 by addressing #3, please review it.

Thanks!
Status changed to Needs work about 1 year ago10:24pm 19 December 2023

I applied patch #4 and I noticed that the patch didin't apply properly due to some files are being rejected, and there are still many PHPCS issues being flagged in my terminal.

➜  viewer git:(master) ✗ curl https://www.drupal.org/files/issues/2023-04-26/codingstandards-3356240-4.patch | patch -p1
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  8351  100  8351    0     0  17988      0 --:--:-- --:--:-- --:--:-- 18434
patching file 'assets/apexcharts/apexcharts.amd.js.LICENSE.txt'
patching file 'assets/chartjs/LICENSE.md'
patching file 'src/Controller/CKEditorPreview.php'
4 out of 5 hunks failed--saving rejects to 'src/Controller/CKEditorPreview.php.rej'
patching file 'src/Form/CKEditorDialogForm.php'
2 out of 3 hunks failed--saving rejects to 'src/Form/CKEditorDialogForm.php.rej'
patching file 'src/Form/Source/ImportForm.php'
patching file 'src/Form/Source/SourceForm.php'
➜  viewer git:(master) ✗ ..
➜  contrib git:(master) ✗ phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml viewer

FILE: /Users/clarksubing-subing/Projects/d9/d9-local/web/modules/contrib/viewer/viewer.info.yml
-------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------------
 1 | WARNING | Remove "project" from the info file, it will be added by drupal.org packaging automatically
 1 | WARNING | Remove "datestamp" from the info file, it will be added by drupal.org packaging automatically
 1 | WARNING | Remove "version" from the info file, it will be added by drupal.org packaging automatically
-------------------------------------------------------------------------------------------------------------


FILE: /Users/clarksubing-subing/Projects/d9/d9-local/web/modules/contrib/viewer/src/Form/CKEditorDialogForm.php
--------------------------------------------------------------------------------------------------------------------------
FOUND 2 ERRORS AND 2 WARNINGS AFFECTING 4 LINES
--------------------------------------------------------------------------------------------------------------------------
  6 | ERROR   | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Core\Ajax\AjaxResponse.
 10 | WARNING | [x] Unused use statement
 11 | WARNING | [x] Unused use statement
 19 | ERROR   | [x] Whitespace found at end of line
--------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 4 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------------------------------------------------


FILE: /Users/clarksubing-subing/Projects/d9/d9-local/web/modules/contrib/viewer/src/Form/Source/SourceForm.php
--------------------------------------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
--------------------------------------------------------------------------------------------------------------
 53 | ERROR | [x] Array indentation error, expected 10 spaces but found 8
 54 | ERROR | [x] Array indentation error, expected 10 spaces but found 8
--------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------------------------------------


FILE: /Users/clarksubing-subing/Projects/d9/d9-local/web/modules/contrib/viewer/src/Form/Source/BaseForm.php
---------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
---------------------------------------------------------------------------------------------------------------------------------------
 8 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Core\Entity\EntityTypeManagerInterface.
---------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/clarksubing-subing/Projects/d9/d9-local/web/modules/contrib/viewer/src/Form/Source/ImportForm.php
--------------------------------------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
--------------------------------------------------------------------------------------------------------------

I didin't include the other issues because it's too many

Assigned to nitin_lama
Comment about 1 year ago →
🇮🇳India nitin_lama India
Comment about 1 year ago →
🇮🇳India nitin_lama India
Providing updated patch for 1.0.x
Comment about 1 year ago →
🇮🇳India nitin_lama India
Please review.
Issue was unassigned.
Status changed to Needs review about 1 year ago12:14pm 26 December 2023
Comment about 1 year ago →
🇮🇳India nitin_lama India
Comment 12 months ago →
bindu r
Hi . the patch #7 applied cleanly, but few warnings are still present.

$ vendor/bin/phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig viewer/

FILE: /home/sites/contributions/viewer/assets/chartjs/LICENSE.md
----------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 3 LINES
----------------------------------------------------------------------
5 | WARNING | Line exceeds 80 characters; contains 432 characters
7 | WARNING | Line exceeds 80 characters; contains 126 characters
9 | WARNING | Line exceeds 80 characters; contains 460 characters
----------------------------------------------------------------------

FILE: /home/sites/contributions/viewer/assets/apexcharts/apexcharts.amd.js.LICENSE.txt
--------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------------
8 | WARNING | Line exceeds 80 characters; contains 83 characters
--------------------------------------------------------------------------------------
Comment 11 months ago →
🇮🇳India Yashaswi18

Comment 11 months ago →

🇮🇳India pray_12

Hi,
Above Patch applied cleanly, but found few warnings.


FILE: /viewer/assets/chartjs/LICENSE.md
--------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 3 LINES
--------------------------------------------------------------------------------------------
 5 | WARNING | Line exceeds 80 characters; contains 432 characters
 7 | WARNING | Line exceeds 80 characters; contains 126 characters
 9 | WARNING | Line exceeds 80 characters; contains 460 characters
--------------------------------------------------------------------------------------------


FILE: /viewer/assets/apexcharts/apexcharts.amd.js.LICENSE.txt
------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------------
 8 | WARNING | Line exceeds 80 characters; contains 83 characters
------------------------------------------------------------------------------------------------------------------

First commit to issue fork.
Merge request !3fix all phpcs issue and created MR → (Open) created by chandansha
Status changed to Needs work 8 months ago8:52am 24 May 2024

Comment 8 months ago →

a.aaronjake

Hi @Chandansha,

I applied MR!3 and found these issues:

FILE: ...terns/Demo-site/drupal-orgissue/web/modules/contrib/viewer/viewer.info.yml
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 1 LINE
--------------------------------------------------------------------------------
 1 | WARNING | Remove "project" from the info file, it will be added by
   |         | drupal.org packaging automatically
 1 | WARNING | Remove "datestamp" from the info file, it will be added by
   |         | drupal.org packaging automatically
 1 | WARNING | Remove "version" from the info file, it will be added by
   |         | drupal.org packaging automatically
--------------------------------------------------------------------------------


FILE: ...-orgissue/web/modules/contrib/viewer/src/Plugin/CKEditor5Plugin/Viewer.php
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
 3 | ERROR | [x] Expected strict_types=1, found strict_types = 1.
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------


FILE: .../Demo-site/drupal-orgissue/web/modules/contrib/viewer/src/ViewerModule.php
--------------------------------------------------------------------------------
FOUND 3 ERRORS AFFECTING 2 LINES
--------------------------------------------------------------------------------
 60 | ERROR | [x] The first parameter of a multi-line function declaration must
    |       |     be on the line after the opening bracket
 64 | ERROR | [x] Multi-line function declarations must have a trailing comma
    |       |     after the last parameter
 64 | ERROR | [x] The closing parenthesis of a multi-line function declaration
    |       |     must be on a new line
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 3 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------

Time: 5.23 secs; Memory: 16MB

Will change status to 'Needs work'
Thank you.

Comment 8 months ago →
arijit acharya
Arijit Acharya → made their first commit to this issue’s fork.

Comment 8 months ago →

arijit acharya

I applied the MR!3 and addressed some of the issues flagged by PHPCS. However, no issues were detected in the 'viewer.info.yml' file. Below, I have listed the errors encountered. Additionally, I have raised an MR to resolve these issues. Please review and merge.

FILE: ...modules/custom/viewer-3356240/src/Controller/CKEditorPreview.php
----------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
----------------------------------------------------------------------
  7 | ERROR | [x] Use statements should be sorted alphabetically. The
    |       |     first wrong one is
    |       |     Drupal\Core\Entity\EntityTypeManagerInterface.
 71 | ERROR | [x] Whitespace found at end of line
----------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ...ules/custom/viewer-3356240/src/Plugin/CKEditor5Plugin/Viewer.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 3 | ERROR | [x] Expected strict_types=1, found strict_types = 1.
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ...eb/modules/custom/viewer-3356240/src/Form/CKEditorDialogForm.php
----------------------------------------------------------------------
FOUND 2 ERRORS AND 2 WARNINGS AFFECTING 4 LINES
----------------------------------------------------------------------
  6 | ERROR   | [x] Use statements should be sorted alphabetically.
    |         |     The first wrong one is
    |         |     Drupal\Core\Ajax\AjaxResponse.
 10 | WARNING | [x] Unused use statement
 11 | WARNING | [x] Unused use statement
 19 | ERROR   | [x] Whitespace found at end of line
----------------------------------------------------------------------
PHPCBF CAN FIX THE 4 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: /app/web/modules/custom/viewer-3356240/src/ViewerModule.php
----------------------------------------------------------------------
FOUND 3 ERRORS AFFECTING 2 LINES
----------------------------------------------------------------------
 60 | ERROR | [x] The first parameter of a multi-line function
    |       |     declaration must be on the line after the opening
    |       |     bracket
 64 | ERROR | [x] Multi-line function declarations must have a
    |       |     trailing comma after the last parameter
 64 | ERROR | [x] The closing parenthesis of a multi-line function
    |       |     declaration must be on a new line
----------------------------------------------------------------------
PHPCBF CAN FIX THE 3 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

Status changed to Needs review 8 months ago11:49am 3 June 2024
Comment 8 months ago →
arijit acharya
Status changed to RTBC 6 months ago8:35am 11 July 2024

Comment 6 months ago →

Esha_Kundu

Hi @Arijit Acharya I have applied the updated MR!3
These are the steps that I have followed

Took the clone of 1.0.x branch which is the reference branch for the tags 1.0.4 which is the latest version of the module.
Executed the command
phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig web/modules/contrib/viewer/

Found phpcs errors

PHP CODE SNIFFER REPORT SUMMARY
----------------------------------------------------------------------
FILE                                                  ERRORS  WARNINGS
----------------------------------------------------------------------
...r/assets/apexcharts/apexcharts.amd.js.LICENSE.txt  0       1
...b/modules/custom/viewer/assets/chartjs/LICENSE.md  0       3
/app/web/modules/custom/viewer/src/ViewerModule.php   4       0
...les/custom/viewer/src/Commands/ViewerCommands.php  1       0
.../custom/viewer/src/Controller/CKEditorPreview.php  2       0
...custom/viewer/src/Controller/ViewerController.php  1       0
.../viewer/src/Controller/ViewerSourceController.php  1       0
/app/web/modules/custom/viewer/src/Entity/Viewer.php  1       0
...modules/custom/viewer/src/Entity/ViewerSource.php  1       0
...tity/AccessControl/ViewerAccessControlHandler.php  1       0
...ccessControl/ViewerSourceAccessControlHandler.php  1       0
.../viewer/src/Entity/Form/BaseContentEntityForm.php  1       0
...ewer/src/Entity/ListBuilder/ViewerListBuilder.php  1       0
...rc/Entity/ListBuilder/ViewerSourceListBuilder.php  1       0
...b/modules/custom/viewer/src/Event/ViewerEvent.php  1       0
...er/src/EventSubscriber/ViewerEventsSubscriber.php  1       0
...les/custom/viewer/src/Form/CKEditorDialogForm.php  2       2
...odules/custom/viewer/src/Form/Source/BaseForm.php  1       0
...odules/custom/viewer/src/Form/Viewer/BaseForm.php  1       0
...tom/viewer/src/ParamConverter/ViewerConverter.php  1       0
...ewer/src/ParamConverter/ViewerPluginConverter.php  1       0
...ewer/src/ParamConverter/ViewerSourceConverter.php  1       0
...rc/ParamConverter/ViewerSourcePluginConverter.php  1       0
.../src/ParamConverter/ViewerTypePluginConverter.php  1       0
...b/modules/custom/viewer/src/Plugin/ViewerBase.php  1       0
...es/custom/viewer/src/Plugin/ViewerCellManager.php  1       0
...odules/custom/viewer/src/Plugin/ViewerManager.php  1       0
.../custom/viewer/src/Plugin/ViewerProcessorBase.php  1       0
...stom/viewer/src/Plugin/ViewerProcessorManager.php  1       0
...les/custom/viewer/src/Plugin/ViewerSourceBase.php  1       0
.../custom/viewer/src/Plugin/ViewerSourceManager.php  1       0
...dules/custom/viewer/src/Plugin/ViewerTypeBase.php  1       0
...es/custom/viewer/src/Plugin/ViewerTypeManager.php  1       0
...es/custom/viewer/src/Plugin/Block/ViewerBlock.php  1       0
...stom/viewer/src/Plugin/CKEditor5Plugin/Viewer.php  1       0
.../Plugin/Field/FieldFormatter/ViewersFormatter.php  1       0
...er/src/Plugin/Field/FieldWidget/ViewersWidget.php  1       0
.../custom/viewer/src/Plugin/Filter/ViewerFilter.php  1       0
...ewer/src/Plugin/viewer/processor/CsvProcessor.php  1       0
...es/custom/viewer/src/Plugin/viewer/source/Ftp.php  1       0
...s/custom/viewer/src/Plugin/viewer/source/SFtp.php  1       0
...custom/viewer/src/Plugin/viewer/source/Upload.php  1       0
...ules/custom/viewer/src/Plugin/viewer/type/Pdf.php  1       0
...les/custom/viewer/src/Plugin/viewer/type/Xlsx.php  1       0
...stom/viewer/src/Plugin/viewer/viewer/ApexLine.php  1       0
...tom/viewer/src/Plugin/viewer/viewer/ApexMixed.php  1       0
...m/viewer/src/Plugin/viewer/viewer/ApexTreemap.php  1       0
...tom/viewer/src/Plugin/viewer/viewer/ChartLine.php  1       0
...om/viewer/src/Plugin/viewer/viewer/ChartMixed.php  1       0
.../custom/viewer/src/Plugin/viewer/viewer/PdfJs.php  1       0
...m/viewer/src/Plugin/viewer/viewer/Spreadsheet.php  1       0
.../custom/viewer/src/Plugin/viewer/viewer/Table.php  1       0
...s/custom/viewer/src/Plugin/viewer/viewer/Tabs.php  1       0
.../web/modules/custom/viewer/src/Services/Batch.php  1       0
/app/web/modules/custom/viewer/src/Services/Cron.php  1       0
...eb/modules/custom/viewer/src/Services/FtpSftp.php  1       0
...ules/custom/viewer/src/Services/Notifications.php  1       0
----------------------------------------------------------------------
A TOTAL OF 60 ERRORS AND 6 WARNINGS WERE FOUND IN 57 FILES
----------------------------------------------------------------------
PHPCBF CAN FIX 62 OF THESE SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

Applied the updated MR!3
Ran the same command again and Found no errors

LGTM

Fix the issues reported by phpcs

Merge Requests

!3Fix the issues reported by phpcs
Open

Comments & Activities

Fix the issues reported by phpcs

Merge Requests

!3Fix the issues reported by phpcsOpen

Comments & Activities

!3Fix the issues reported by phpcs
Open