- 🇩🇪Germany mxh Offenburg
Formatting is still not following Drupal coding standards.
- 🇮🇹Italy apaderno Brescia, 🇮🇹
The issue summary should always describe what the issue is trying to fix and, in the case, of coding standards issues, show which command has been used, which arguments have been used, and which report that command shown.
- First commit to issue fork.
- Status changed to Needs review
almost 2 years ago 4:53pm 7 March 2023 - Status changed to Needs work
almost 2 years ago 5:56pm 7 March 2023 - 🇧🇷Brazil elber Brazil
Hi I ran the follow commands
phpcs --standard=Drupal --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml
phpcs --standard=DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml
after that I made the changes I didn't see phpcs errors anymore, @mxh could you please show me which the errors you are seeing? - 🇮🇹Italy apaderno Brescia, 🇮🇹
@elber See the previous comments. The issue summary is not even reporting which
phpcs
coding standards have been used. - 🇧🇷Brazil elber Brazil
Hi I saw the previous comments and I didn't understand what is wrong with this issue because phpcs errors has been already fixed, please can you tell me what is wrong?
- 🇩🇪Germany mxh Offenburg
By looking into the suggested merge request (which already takes time BTW), I can immedtiately see that your changes are not following Drupal coding standards. I don't take the time to tell what exactly is wrong. I can assure you that the coding in the merge request is wrong regards Drupal coding standards. Read the guidelines of Drupal coding standards, understand them, look at your suggested merge request and then you'll see what's wrong.
- 🇧🇷Brazil elber Brazil
Hi I ran drupal phpcs commands to check phpcs erros as I said in the comment #23 and I didn't have erros in output, when do you find the phpcs errors you can let me know it wil be a pleasure keeping working in this issue, thank you!
- 🇮🇹Italy apaderno Brescia, 🇮🇹
Since a MR has been provided, it would be better to keep using that MR.
- 🇮🇳India zkhan.aamir
Hi,
The patch #35 applied succesfully.
Admin@DESKTOP-252TO6V MINGW64 ~/Desktop/projects/drupal/web/modules/contrib/eca_cm (1.0.x) $ curl https://www.drupal.org/files/issues/2023-11-29/phpcs-fixes-3302844-35.patch | patch -p1 % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 100 13061 100 13061 0 0 13553 0 --:--:-- --:--:-- --:--:-- 13576 patching file src/Controller/CoreModeller.php patching file src/Form/EcaForm.php patching file src/Form/EcaPluginDeleteForm.php patching file src/Form/EcaPluginForm.php
But still there are some errors
Admin@DESKTOP-252TO6V MINGW64 ~/Desktop/projects/drupal/web/modules/contrib $ phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,info,txt,md,js,yml eca_cm/ FILE: C:\Users\Admin\Desktop\projects\drupal\web\modules\contrib\eca_cm\src\Form\EcaForm.php -------------------------------------------------------------------------------------------------------------------------------------- FOUND 1 ERROR AFFECTING 1 LINE -------------------------------------------------------------------------------------------------------------------------------------- 587 | ERROR | The array declaration extends to column 82 (the limit is 80). The array content should be split up over multiple lines -------------------------------------------------------------------------------------------------------------------------------------- FILE: C:\Users\Admin\Desktop\projects\drupal\web\modules\contrib\eca_cm\src\Form\EcaPluginForm.php --------------------------------------------------------------------------------------------------------------------------------------- FOUND 7 ERRORS AFFECTING 7 LINES --------------------------------------------------------------------------------------------------------------------------------------- 314 | ERROR | The array declaration extends to column 89 (the limit is 80). The array content should be split up over multiple lines 714 | ERROR | The array declaration extends to column 88 (the limit is 80). The array content should be split up over multiple lines 717 | ERROR | The array declaration extends to column 111 (the limit is 80). The array content should be split up over multiple lines 747 | ERROR | The array declaration extends to column 120 (the limit is 80). The array content should be split up over multiple lines 876 | ERROR | The array declaration extends to column 104 (the limit is 80). The array content should be split up over multiple lines 879 | ERROR | The array declaration extends to column 108 (the limit is 80). The array content should be split up over multiple lines 882 | ERROR | The array declaration extends to column 105 (the limit is 80). The array content should be split up over multiple lines --------------------------------------------------------------------------------------------------------------------------------------- Time: 1.1 secs; Memory: 20MB
- Status changed to Needs review
11 months ago 2:15pm 19 January 2024 - 🇮🇳India sakthi_dev
Leaving 314 as it is because it is a if clause. Please review.
- Status changed to Needs work
11 months ago 2:47pm 19 January 2024 - 🇮🇹Italy apaderno Brescia, 🇮🇹
- public function __construct(Modellers $modeller_services, Event $event_manager, Condition $condition_manager, Action $action_manager, FormBuilderInterface $form_builder) { + public function __construct( + Modellers $modeller_services, + Event $event_manager, + // Corrected variable name. + Condition $condition_manager, + Action $action_manager, + FormBuilderInterface $form_builder + ) {
Function/method declarations are written on a single line. There is no need to add a comment to describe what change has been introduced.
$this->eventManager = $event_manager; + // Updated variable assignment. $this->conditionManager = $condition_manager;
That comment is not necessary and must be removed.
+ /** + * {@inheritdoc} + */ public function addEvent(Eca $eca, string $eca_event_plugin): array {
That method is not inherited from the parent class. Its documentation comment must be different.
- foreach ($plugins['conditions.' . $config_key] as $plugin) {} + foreach ($plugins['conditions.' . $config_key] as $plugin) { + }
An empty loop can be simply be removed.
Since an issue fork has been created, let's continue to use it.
- Status changed to Needs review
11 months ago 5:55pm 1 February 2024 Hi, The changes suggested in comment #38 has been incorporated in the patch #39.
Thankyou.- Status changed to Needs work
11 months ago 10:48am 10 February 2024 - 🇮🇹Italy apaderno Brescia, 🇮🇹
Since a issue fork has been already created, let's keep using it.
- 🇮🇹Italy apaderno Brescia, 🇮🇹
apaderno → changed the visibility of the branch coding_standard to hidden.
- Status changed to Needs review
11 months ago 1:26pm 10 February 2024 - 🇮🇳India zkhan.aamir
Hi,
MR #43 applied successfully.
Admin@DESKTOP-252TO6V MINGW64 ~/Desktop/projects/drupal/web/modules/contrib/eca_cm (1.0.x) $ curl https://git.drupalcode.org/project/eca_cm/-/merge_requests/7.diff | patch -p1 % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 100 16264 0 16264 0 0 34845 0 --:--:-- --:--:-- --:--:-- 35051 patching file src/Controller/CoreModeller.php patching file src/Form/EcaPluginDeleteForm.php patching file src/Form/EcaPluginForm.php
There are still some errors
Admin@DESKTOP-252TO6V MINGW64 ~/Desktop/projects/drupal/web/modules/contrib $ phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,info,txt,md,js,yml eca_cm/ FILE: C:\Users\Admin\Desktop\projects\drupal\web\modules\contrib\eca_cm\src\Form\EcaForm.php ---------------------------------------------------------------------------------------------------------------------------------------- FOUND 1 ERROR AND 1 WARNING AFFECTING 2 LINES ---------------------------------------------------------------------------------------------------------------------------------------- 587 | ERROR | The array declaration extends to column 82 (the limit is 80). The array content should be split up over multiple lines 955 | WARNING | Avoid backslash escaping in translatable strings when possible, use '' quotes instead ---------------------------------------------------------------------------------------------------------------------------------------- FILE: C:\Users\Admin\Desktop\projects\drupal\web\modules\contrib\eca_cm\src\Form\EcaPluginForm.php --------------------------------------------------------------------------------------------------------------------------------------- FOUND 3 ERRORS AFFECTING 3 LINES --------------------------------------------------------------------------------------------------------------------------------------- 314 | ERROR | The array declaration extends to column 89 (the limit is 80). The array content should be split up over multiple lines 1071 | ERROR | Doc comment is empty 1082 | ERROR | Doc comment is empty --------------------------------------------------------------------------------------------------------------------------------------- Time: 1.01 secs; Memory: 20MB
- First commit to issue fork.
- 🇮🇳India pray_12
Hi,
The errors and warnings from #45 have been successfully addressed;
however, there are still some remaining errors and warnings that need attention.FILE: .../eca_cm/src/Form/EcaForm.php -------------------------------------------------------------------------------------------------------------------------------------- FOUND 1 ERROR AFFECTING 1 LINE -------------------------------------------------------------------------------------------------------------------------------------- 587 | ERROR | The array declaration extends to column 82 (the limit is 80). The array content should be split up over multiple lines -------------------------------------------------------------------------------------------------------------------------------------- FILE: .../eca_cm/src/Form/EcaPluginForm.php -------------------------------------------------------------------------------------------------------------------------------------- FOUND 1 ERROR AFFECTING 1 LINE -------------------------------------------------------------------------------------------------------------------------------------- 314 | ERROR | The array declaration extends to column 89 (the limit is 80). The array content should be split up over multiple lines --------------------------------------------------------------------------------------------------------------------------------------
- 🇮🇹Italy apaderno Brescia, 🇮🇹
Given that the code in the EcaPluginForm.php file is the following line and that control structure conditions should not be wrapped into multiple lines → , it is not possible to change it.
if (isset($elements['#type']) && in_array($elements['#type'], ['number', 'email'], TRUE)) { $elements['#original_form_type'] = $elements['#type']; $elements['#type'] = 'textfield'; }
- Status changed to RTBC
7 months ago 8:02am 5 June 2024 - 🇮🇳India dev16.addweb
Hello,
I applied the patch provided in !7 for the version 1.0.x-dev, the patch applied successfully.