🇮🇳India @Nayana Ramakrishnan

Account created on 29 November 2019, over 4 years ago
#

Recent comments

🇮🇳India Nayana Ramakrishnan

Verified the patch in Drupal version 10.1.x and Gin Admin Theme version 8.x-3.x. The patch applied cleanly and now the Diff changes are in black font so it's better readable compared to previous implementation. I have added the before and after screenshots for reference.

🇮🇳India Nayana Ramakrishnan

Also, I have reverted the changes in line 280 as per your comment. So can we ignore WARNING | [ ] Unused variable $span_lang. in such cases?

🇮🇳India Nayana Ramakrishnan

@benjifisher I tested it only using the command phpcs --standard="Drupal,DrupalPractice" --extensions=php,module,inc,install,test,profile,theme,css,js,info,txt,md,yml,twig. That's why I put Need RTBC+1 in the comment. I have seen such comments from reviewers in other tickets also. Will avoid adding such comments if it is confusing.

🇮🇳India Nayana Ramakrishnan

Hi @edyuenyw, I verified your patch and I can confirm that it fixes the issue mentioned in the ticket. But I don’t know why it’s showing test failure. Attaching before and after screenshots for reference.

🇮🇳India Nayana Ramakrishnan

@larowlan Have added the details related to composer dependency and the links under installation section. Kindly review.

🇮🇳India Nayana Ramakrishnan

This module already has a hook_help function. In the latest patch, only the text which is displayed in the help page is changed. Is it really required to change that text? If so, then it will be good if we can get a confirmation about the same.

🇮🇳India Nayana Ramakrishnan

In the latest branch of this module i.e., 10.0.x, the README file is already updated and it is as per the README.md template . So I think no need to change the content of README file again with this ticket.

🇮🇳India Nayana Ramakrishnan

Verified the changes in the branch. There were some format issues which I have corrected in the new commit. Also, updated few more details and created MR!1. Please review. Thanks

🇮🇳India Nayana Ramakrishnan

nayana_mvr made their first commit to this issue’s fork.

🇮🇳India Nayana Ramakrishnan

Verified the patch #6. It fixes all the errors mentioned in the ticket and also addresses the changes mentioned in #5. Need RTBC+1.

🇮🇳India Nayana Ramakrishnan

Verified MR!2. The patch applied cleanly and all the phpcs issues mentioned in the ticket are fixed now. Need RTBC+1.

🇮🇳India Nayana Ramakrishnan

Verified the changes and created the MR!37. I have updated the maintainers details also in README file and committed them to the same branch. Everything else looks good and it follows the pattern in README.md template

🇮🇳India Nayana Ramakrishnan

Verified the MR!1. The patch applied cleanly and all the coding standard issues are fixed now. Need RTBC+1.

🇮🇳India Nayana Ramakrishnan

Verified the MR!239 on Drupal version 10.1.x and Gin Admin Theme version 8.x-3.x. The patch applied cleanly and now the icons are not distorted. Checked in Claro and Olivero theme also but those icons are not used in those themes. So I think this issue is only with Gin theme. I have attached before and after screenshots for reference.

🇮🇳India Nayana Ramakrishnan

@saganakat Please make sure to change the status of the ticket to 'Needs Review' if you are uploading a patch or raising MR. It will be easy for the community to review them quickly.

Verified the patch #2 on Drupal version 10.1.x and Gin Admin Theme version 8.x-3.x. The patch applied cleanly and it fixes the overlapping issue in horizontal tabs. I have attached the before and after screenshots for reference. Need RTBC+1 so moving to Needs Review.

🇮🇳India Nayana Ramakrishnan

Verified MR!7. The patch applied cleanly with a warning message:

7.diff:10: trailing whitespace.
Layout Builder Related Events Blocks is a Helper module for the custom 
Checking patch README.md...
Checking patch lb_related_events_blocks.install...
Checking patch lb_related_events_blocks.module...
Applied patch README.md cleanly.
Applied patch lb_related_events_blocks.install cleanly.
Applied patch lb_related_events_blocks.module cleanly.
warning: 1 line adds whitespace errors.

Also, there are few more errors to be fixed.

FILE: .../contrib/lb_related_events_blocks/lb_related_events_blocks.info.yml
-----------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 3 LINES
-----------------------------------------------------------------------------------------------------------------------------
 10 | WARNING | All dependencies must be prefixed with the project name, for example "drupal:"
 11 | WARNING | All dependencies must be prefixed with the project name, for example "drupal:"
 12 | WARNING | All dependencies must be prefixed with the project name, for example "drupal:"
-----------------------------------------------------------------------------------------------------------------------------


FILE: .../contrib/lb_related_events_blocks/assets/css/lb-related-events-node.css
---------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
---------------------------------------------------------------------------------------------------------------------------------
 1 | WARNING | File appears to be minified and cannot be processed
---------------------------------------------------------------------------------------------------------------------------------


FILE: .../contrib/lb_related_events_blocks/assets/css/lb-related-events-block.css
----------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------------------------------------
 1 | WARNING | File appears to be minified and cannot be processed
----------------------------------------------------------------------------------------------------------------------------------

Time: 174ms; Memory: 12MB
🇮🇳India Nayana Ramakrishnan

Verified MR!1. The patch applied cleanly and it fixes all the php coding standard issues. Need RTBC+1.

🇮🇳India Nayana Ramakrishnan

Verified the MR!1 on Drupal version 9.5.x and OpenChurch Theme version 3.0.x. The patch applied cleanly and now an error message is displayed when an invalid color code is given n the 'Link Color' field. I have added the before and after screen recordings for reference.
Just one more concern:- The text font color of that field is always white. Is it expected to be like that? Because if we put some invalid color in that field and try save the configuration, then the background color of that field will be white which makes it difficult to read the text given in that field (Please refer screen recording for more clarity.)

🇮🇳India Nayana Ramakrishnan

MR!1 is created for this ticket. Please review.
Thanks.

🇮🇳India Nayana Ramakrishnan

Verified the patch #7. The patch applied cleanly and it fixes all the errors mentioned in the ticket. Need RTBC+1

🇮🇳India Nayana Ramakrishnan

Verified the patch #17 on Drupal version 10.1.x and Field Permission version 8.x-1.x. The patch applied cleanly and except the following errors, all other coding standard issues are fixed.

FILE: .../contrib/field_permissions/field_permissions.module
--------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------------------------------------------------------------------------------------
 151 | ERROR | All functions defined in a module file must be prefixed with the module's name, found "field_permission_field_config_edit_form_submit" but
     |       | expected "field_permissions_field_permission_field_config_edit_form_submit"
--------------------------------------------------------------------------------------------------------------------------------------------------------------


FILE: .../contrib/field_permissions/src/Plugin/FieldPermissionType/Manager.php
--------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------------------------------------------------------
 56 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
--------------------------------------------------------------------------------------------------------------------------------

Time: 796ms; Memory: 12MB
🇮🇳India Nayana Ramakrishnan

Verified the MR!3603 on Drupal version 10.1.x. The patch applied cleanly and now the status message on comment edit is changed to 'Your comment has been updated.' I have added the before and after screenshots for reference. Need RTBC+1.

🇮🇳India Nayana Ramakrishnan

Verified the patch #14. The patch applied with a warning message:

3328568-14.patch:108: trailing whitespace.
  
Checking patch src/Entity/Profile.php...
Checking patch src/Form/ProfileTypeForm.php...
Checking patch src/Plugin/Field/FieldWidget/ProfileFormWidget.php...
Checking patch tests/src/Functional/ProfileDefaultTest.php...
Checking patch tests/src/Functional/ProfileRegisterFormTest.php...
Checking patch tests/src/Functional/ProfileTypeTest.php...
Applied patch src/Entity/Profile.php cleanly.
Applied patch src/Form/ProfileTypeForm.php cleanly.
Applied patch src/Plugin/Field/FieldWidget/ProfileFormWidget.php cleanly.
Applied patch tests/src/Functional/ProfileDefaultTest.php cleanly.
Applied patch tests/src/Functional/ProfileRegisterFormTest.php cleanly.
Applied patch tests/src/Functional/ProfileTypeTest.php cleanly.
warning: 1 line adds whitespace errors.

Also, the warnings related to README.md file was not there in the patch. So I have fixed the above warning and the warnings in the README file and created a new MR. Please review. Thanks.

🇮🇳India Nayana Ramakrishnan

nayana_mvr made their first commit to this issue’s fork.

🇮🇳India Nayana Ramakrishnan

Verified the patch #7. There were some format issues. Corrected them and created an MR for this ticket with all the changes. Please review.
Thanks.

🇮🇳India Nayana Ramakrishnan

Verified the MR!7. The patch applied cleanly and now the help button is available in the module listing page. Regarding the content, don't we need to add the configuration steps also in the help page? I think it will be useful for the users.

🇮🇳India Nayana Ramakrishnan

Verified MR!4 and it looks good to me. It follows the pattern in README.md template .

🇮🇳India Nayana Ramakrishnan

Verified the patch #6. The patch applied cleanly and all the coding standard issues mentioned in the ticket are fixed.

🇮🇳India Nayana Ramakrishnan

Verified the MR!19 . The patch applied cleanly and now the help button for the module is available in the module listing page. Attached screenshots for reference. Need confirmation about the content to be displayed in the help page.

🇮🇳India Nayana Ramakrishnan

Created patch for the issue. Please review.
Thanks

🇮🇳India Nayana Ramakrishnan

Verified the MR!238 on Drupal version 10.1.x and Gin Admin Theme version 8.x-3.x. The patch applied cleanly and tested it with media modal dialog box. Now the modal box is fixed in the centre and I have added the before and after screenshots for reference. Need RTBC+1.

🇮🇳India Nayana Ramakrishnan

Implemented hook_help function and created the MR. Please review.
Thanks

🇮🇳India Nayana Ramakrishnan

Created MR!40 for the ticket. Please review.
Thanks.

🇮🇳India Nayana Ramakrishnan

Created MR!40 for the ticket. Please review.
Thanks.

🇮🇳India Nayana Ramakrishnan

Updated some minor fixes related to format and committed to MR!30. Please review.
Thanks.

🇮🇳India Nayana Ramakrishnan

nayana_mvr made their first commit to this issue’s fork.

🇮🇳India Nayana Ramakrishnan

Verified the patch #2 on Drupal version 10.1.x and Re-assign user content/media version 1.0.x . The patch applied cleanly and all the issues mentioned in the ticket are fixed. Need RTBC+1.

🇮🇳India Nayana Ramakrishnan

Verified the patch #5 and found few format issues. Corrected them and created MR!73 with all the change. Please review.
Thanks.

🇮🇳India Nayana Ramakrishnan

nayana_mvr made their first commit to this issue’s fork.

🇮🇳India Nayana Ramakrishnan

Verified the MR!4 and found sone coding standard issues. Fixed them in the new commit. Other than that the MR looks good. It follows the pattern in README.md Template .

🇮🇳India Nayana Ramakrishnan

Verified the patch on Drupal version 10.1.x and Gin Layout Builder 1.0.x. The patch applied cleanly and now the config button is available in the module listing page and on clicking that it correctly redirects to the Gin Layout Builder Settings page. Attached before and after screenshots for reference. Moving to RTBC.

🇮🇳India Nayana Ramakrishnan

Verified the patch #8. The patch applied cleanly but its showing a new warning message now:

FILE: .../contrib/json_ld_schema_ui/src/Plugin/DataType/SchemaOverrides.php
-----------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-----------------------------------------------------------------------------------------------------------------------------
 41 | WARNING | Unused variable $override.
-----------------------------------------------------------------------------------------------------------------------------

Time: 665ms; Memory: 14MB

All other issues are fixed.

🇮🇳India Nayana Ramakrishnan

Verified the patch #31 on Drupal version 10.1.x. The patch applied cleanly and Luxembourgish language is now available in the language option. also verified that the user is able to add the language and tried to translate the text 'Content' to that language. Added screenshots for reference.

🇮🇳India Nayana Ramakrishnan

Verified the MR!2 on Drupal version 10.1.x. All the coding standard errors mentioned in the ticket are fixed

🇮🇳India Nayana Ramakrishnan

Verified the patch #64 on Drupal version 10.1.x with Olivero theme. Patch applied cleanly and can confirm that the issue is fixed for Olivero theme. Screenshot attached for reference.

🇮🇳India Nayana Ramakrishnan

Updated REAMD.md and created MR!20 for the ticket. Please review.
Thanks.

🇮🇳India Nayana Ramakrishnan

There were some format issues. Corrected them in the new. Please review.
Thanks.

🇮🇳India Nayana Ramakrishnan

@Rashmisoni @apaderno Thanks for pointing out that. I was verifying the patch using Drupal and DrupalPractice as standard. That's why it was showing more errors. Verified the patch #7 again for issues reported by phpcs using Drupal as standard alone and can confirm that all the issues are fixed.

🇮🇳India Nayana Ramakrishnan

Corrected some formatting issues and updated maintainers details in the new commit. Please review.
Thanks

🇮🇳India Nayana Ramakrishnan

Created README.md file following the pattern in README.md template and created MR!22. Please review.
Thanks.

🇮🇳India Nayana Ramakrishnan

We could be using README.md instead of README.txt to make use of the nicer rendering via markdown.
Now that we've moved to GitLab, we should probably use Markdown instead of plain .txt files for our documentation.

🇮🇳India Nayana Ramakrishnan

Reviewed the patch #2. Patch applied cleanly and except the Readme warning, all other issues are fixed.

FILE: .../contrib/entityreference_dragdrop/README.txt
------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------
 1 | ERROR | File contains UTF-8 byte order mark, which may corrupt your application
------------------------------------------------------------------------------------------------------

Time: 170ms; Memory: 12MB
🇮🇳India Nayana Ramakrishnan

Created MR!1 for this ticket. Please review.
Thanks.

🇮🇳India Nayana Ramakrishnan

nayana_mvr made their first commit to this issue’s fork.

🇮🇳India Nayana Ramakrishnan

Verified patch #63 on Drupal version 10.1.x. Patch applied cleanly but it doesn't fix the original issue mentioned in the ticket. Attached screenshots for reference.

🇮🇳India Nayana Ramakrishnan

Verified MR!8. Except the following warning, all other errors are fixed:

FILE: /contrib/send_emails/src/EmailApi.php
-----------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-----------------------------------------------------------------------------------------------
 158 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
-----------------------------------------------------------------------------------------------

Time: 368ms; Memory: 14MB
🇮🇳India Nayana Ramakrishnan

In the patch #4, the new README.md file is not available. So I did the changes again and created the MR!56 for this ticket. Please review.
Thanks.

🇮🇳India Nayana Ramakrishnan

Created MR!19 for this ticket. Please review.
Thanks.

🇮🇳India Nayana Ramakrishnan

Verified the patch #17 on Drupal version 101.x and Charts version 5.0.x. There were 2 warning messages while applying the patch:

3339824-phpcs-17.patch:203: trailing whitespace.
      "clean-chartjs": "chmod +x scripts/clean-chartjs.sh && 
3339824-phpcs-17.patch:217: trailing whitespace.
    composer require --prefer-dist npm-asset/chart.js:^3.3 
...
warning: 2 lines add whitespace errors.

After applying the patch, there are still some more errors. Please refer the files attached.

🇮🇳India Nayana Ramakrishnan

Verified MR!3568 and tested it on Drupal version 10.1.x. The patch works fine and I have added the before and after screenshots for reference.

🇮🇳India Nayana Ramakrishnan

Reviewed MR!2. The patch applied cleanly and the new README.md file follows the format as per README.md template . RTBC

🇮🇳India Nayana Ramakrishnan

Created a new MR!10 fixing all the errors mentioned in the ticket.
@vishaljd I can see that in the MR!9, there are some changes in advpoll.services.ymlbut I couldn't find any error in that file before applying the patch so I have excluded that change in the new MR.

Please review. Thanks.

🇮🇳India Nayana Ramakrishnan

Verified MR!9 on Drupal version 9.5.x and Advanced Poll version 8.x - 1.x. Patch applied cleanly but errors in advpoll.install and advpoll.module are not there in the MR. So its showing the follwing errors even after applying the patch:

FILE: ../contrib/advpoll/advpoll.install
--------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 7 ERRORS AND 4 WARNINGS AFFECTING 7 LINES
--------------------------------------------------------------------------------------------------------------------------------------------------------------
  1 | ERROR   | [x] Missing file doc comment
  7 | WARNING | [ ] Format should be "* Implements hook_foo().", "* Implements hook_foo_BAR_ID_bar() for xyz_bar().",, "* Implements hook_foo_BAR_ID_bar()
    |         |     for xyz-bar.html.twig.", "* Implements hook_foo_BAR_ID_bar() for xyz-bar.tpl.php.", or "* Implements hook_foo_BAR_ID_bar() for block
    |         |     templates."
  7 | ERROR   | [x] Doc comment short description must start with a capital letter
  7 | ERROR   | [x] Doc comment short description must end with a full stop
 10 | ERROR   | [x] Inline comments must start with a capital letter
 42 | WARNING | [ ] Format should be "* Implements hook_foo().", "* Implements hook_foo_BAR_ID_bar() for xyz_bar().",, "* Implements hook_foo_BAR_ID_bar()
    |         |     for xyz-bar.html.twig.", "* Implements hook_foo_BAR_ID_bar() for xyz-bar.tpl.php.", or "* Implements hook_foo_BAR_ID_bar() for block
    |         |     templates."
 42 | ERROR   | [x] Doc comment short description must start with a capital letter
 42 | ERROR   | [x] Doc comment short description must end with a full stop
 45 | ERROR   | [x] Inline comments must start with a capital letter
 71 | WARNING | [ ] Unused variable $entity.
 84 | WARNING | [ ] Unused variable $entity.
--------------------------------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 7 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------------------------------------------------------------------------------------


FILE: ../contrib/advpoll/advpoll.module
------------------------------------------------------------------------------------------------------------------------------
FOUND 5 ERRORS AND 11 WARNINGS AFFECTING 13 LINES
------------------------------------------------------------------------------------------------------------------------------
  1 | ERROR   | [x] Missing file doc comment
 11 | WARNING | [ ] Line exceeds 80 characters; contains 111 characters
 12 | WARNING | [ ] Line exceeds 80 characters; contains 113 characters
 12 | ERROR   | [x] Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses
 29 | WARNING | [x] A comma should follow the last multiline array item. Found: 'content'
 36 | WARNING | [x] A comma should follow the last multiline array item. Found: TRUE
 38 | WARNING | [x] A comma should follow the last multiline array item. Found: 'content'
 45 | WARNING | [x] A comma should follow the last multiline array item. Found: TRUE
 47 | WARNING | [x] A comma should follow the last multiline array item. Found: 'content'
 53 | WARNING | [x] A comma should follow the last multiline array item. Found: 'content'
 59 | WARNING | [x] A comma should follow the last multiline array item. Found: 'content'
 68 | WARNING | [ ] Line exceeds 80 characters; contains 101 characters
 68 | ERROR   | [x] Inline comments must start with a capital letter
 68 | ERROR   | [x] Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses
 70 | ERROR   | [x] Short array syntax must be used to define arrays
 92 | WARNING | [ ] Unused variable $poll.
------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 12 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------------------------------------------------

Time: 268ms; Memory: 12MB
🇮🇳India Nayana Ramakrishnan

I just tried to fix the remaining issues and created the MR!1 with all the changes. Please review.
Thanks.

🇮🇳India Nayana Ramakrishnan

Verified the patch #2 on Drupal version 10.1.x and Type Style version 8.x-1.x.
Command used is Vendor/bin/phpcs --standard="Drupal,DrupalPractice" --extensions=php,module,inc,install,test,profile,theme,info,txt,md,yml modules/contrib/type_style

Patch applied cleanly but there are few more errors.

FILE: .../contrib/type_style/type_style.module
------------------------------------------------------------------------------------------------
FOUND 5 ERRORS AFFECTING 4 LINES
------------------------------------------------------------------------------------------------
  49 | ERROR | [ ] Type hint "array" missing for &$form
  49 | ERROR | [ ] Type hint "array" missing for $settings
  62 | ERROR | [x] Use null coalesce operator instead of ternary operator.
  68 | ERROR | [x] Use null coalesce operator instead of ternary operator.
 126 | ERROR | [x] Use null coalesce operator instead of ternary operator.
------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 3 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------------------


FILE: .../contrib/type_style/modules/type_style_moderation/type_style_moderation.install
------------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------------------------------------
 4 | ERROR | [x] Doc comment short description must be on the first line
------------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------------------------------------------------------------


FILE: .../contrib/type_style/modules/type_style_moderation/type_style_moderation.info.yml
-------------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
-------------------------------------------------------------------------------------------------------------------------------------------
 1 | WARNING | Remove "version" from the info file, it will be added by drupal.org packaging automatically
 9 | WARNING | All dependencies must be prefixed with the project name, for example "drupal:"
-------------------------------------------------------------------------------------------------------------------------------------------


FILE: .../contrib/type_style/modules/type_style_moderation/type_style_moderation.module
-----------------------------------------------------------------------------------------------------------------------------------------
FOUND 5 ERRORS AFFECTING 5 LINES
-----------------------------------------------------------------------------------------------------------------------------------------
   4 | ERROR | [x] Doc comment short description must be on the first line
  30 | ERROR | [x] Use null coalesce operator instead of ternary operator.
  61 | ERROR | [x] Use null coalesce operator instead of ternary operator.
  99 | ERROR | [x] Use null coalesce operator instead of ternary operator.
 131 | ERROR | [x] Use null coalesce operator instead of ternary operator.
-----------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 5 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-----------------------------------------------------------------------------------------------------------------------------------------
🇮🇳India Nayana Ramakrishnan

MR!19 created for the ticket. Please review.
Thanks

Production build 0.69.0 2024